Fix SkipScan with varchar column

This patch changes the operator lookup in SkipScan to fall back
to the type of the operator class when it finds no direct match
for column type but the type is binary compatible with the type
of the operator class. This is required for SkipScan to work with
character data column that are not of type text since the
operator family will be text_ops which has no operators for
varchar defined.

Fixes #3720

CHANGELOG.md

@@ -6,15 +6,16 @@ accidentally triggering the load of a previous DB version.**
 
 ## Unreleased
 
+**Features**
+* #3598 Improve evaluation of stable functions such as now() on access node
+
 **Bugfixes**
 * #3580 Fix memory context bug executing TRUNCATE
 * #3654 Fix index attnum mapping in reorder_chunk
 * #3661 Fix SkipScan path generation with constant DISTINCT column
 * #3708 Fix crash in get_aggsplit
 * #3709 Fix ordered append pathkey check
-
-**Improvements**
-* #3598 Improve evaluation of stable functions such as now() on access node
+* #3728 Fix SkipScan with varchar column
 
 **Thanks**
 * @binakot and @sebvett for reporting an issue with DISTINCT queries

tsl/src/nodes/skip_scan/planner.c

@@ -18,6 +18,7 @@
 #include <optimizer/planmain.h>
 #include <optimizer/restrictinfo.h>
 #include <optimizer/tlist.h>
+#include <parser/parse_coerce.h>
 #include <parser/parsetree.h>
 #include <rewrite/rewriteManip.h>
 #include <utils/syscache.h>
@@ -536,6 +537,7 @@ build_skip_qual(PlannerInfo *root, SkipScanPath *skip_scan_path, IndexPath *inde
 	Oid column_type = exprType((Node *) var);
 	Oid column_collation = get_typcollation(column_type);
 	TypeCacheEntry *tce = lookup_type_cache(column_type, 0);
+	bool need_coerce = false;
 
 	/*
 	 * Skipscan is not applicable for the following case:
@@ -568,24 +570,51 @@ build_skip_qual(PlannerInfo *root, SkipScanPath *skip_scan_path, IndexPath *inde
 		strategy =
 			(strategy == BTLessStrategyNumber) ? BTGreaterStrategyNumber : BTLessStrategyNumber;
 	}
+	Oid opcintype = info->opcintype[idx_key];
 
 	Oid comparator =
 		get_opfamily_member(info->sortopfamily[idx_key], column_type, column_type, strategy);
-	if (!OidIsValid(comparator))
-		return false; /* cannot use this index */
 
-	Const *prev_val = makeNullConst(column_type, -1, column_collation);
-	Var *current_val = makeVar(info->rel->relid /*varno*/,
-							   var->varattno /*varattno*/,
-							   column_type /*vartype*/,
-							   -1 /*vartypmod*/,
-							   column_collation /*varcollid*/,
-							   0 /*varlevelsup*/);
+	/* If there is no exact operator match for the column type we have here check
+	 * if we can coerce to the type of the operator class. */
+	if (!OidIsValid(comparator))
+	{
+		if (IsBinaryCoercible(column_type, opcintype))
+		{
+			comparator =
+				get_opfamily_member(info->sortopfamily[idx_key], opcintype, opcintype, strategy);
+			if (!OidIsValid(comparator))
+				return false;
+			need_coerce = true;
+		}
+		else
+			return false; /* cannot use this index */
+	}
+
+	Const *prev_val = makeNullConst(need_coerce ? opcintype : column_type, -1, column_collation);
+	Expr *current_val = (Expr *) makeVar(info->rel->relid /*varno*/,
+										 var->varattno /*varattno*/,
+										 column_type /*vartype*/,
+										 -1 /*vartypmod*/,
+										 column_collation /*varcollid*/,
+										 0 /*varlevelsup*/);
+
+	if (need_coerce)
+	{
+		CoerceViaIO *coerce = makeNode(CoerceViaIO);
+		coerce->arg = current_val;
+		coerce->resulttype = opcintype;
+		coerce->resultcollid = column_collation;
+		coerce->coerceformat = COERCE_IMPLICIT_CAST;
+		coerce->location = -1;
+
+		current_val = &coerce->xpr;
+	}
 
 	Expr *comparison_expr = make_opclause(comparator,
 										  BOOLOID /*opresulttype*/,
 										  false /*opretset*/,
-										  &current_val->xpr /*leftop*/,
+										  current_val /*leftop*/,
 										  &prev_val->xpr /*rightop*/,
 										  InvalidOid /*opcollid*/,
 										  info->indexcollations[idx_key] /*inputcollid*/);
@@ -654,7 +683,8 @@ fix_indexqual(IndexOptInfo *index, RestrictInfo *rinfo, AttrNumber scankey_attno
 
 	/* fix_indexqual_operand */
 	Assert(index->indexkeys[scankey_attno - 1] != 0);
-	Var *node = linitial_node(Var, op->args);
+	Var *node = linitial_node(Var, pull_var_clause(linitial(op->args), 0));
+
 	Assert(((Var *) node)->varno == index->rel->relid &&
 		   ((Var *) node)->varattno == index->indexkeys[scankey_attno - 1]);
 

tsl/test/expected/plan_skip_scan-12.out

@@ -3949,3 +3949,26 @@ SELECT DISTINCT ON (a) * FROM i3629 WHERE a in (2) ORDER BY a ASC, time DESC;
  2 | Fri Mar 20 00:00:00 2020 PDT
 (1 row)
 
+-- #3720 skipscan not being used on varchar column
+CREATE TABLE i3720(time timestamptz not null,data varchar);
+SELECT table_name FROM create_hypertable('i3720','time');
+ table_name 
+------------
+ i3720
+(1 row)
+
+INSERT INTO i3720
+SELECT time, (array['Yes', 'No', 'Maybe'])[floor(random() * 3 + 1)]
+FROM generate_series('2000-01-01'::timestamptz,'2000-01-03'::timestamptz, '10 minute'::interval) AS g1(time);
+CREATE INDEX ON i3720(data, time);
+ANALYZE i3720;
+:PREFIX SELECT DISTINCT ON(data) * FROM i3720;
+                                                      QUERY PLAN                                                      
+----------------------------------------------------------------------------------------------------------------------
+ Unique (actual rows=3 loops=1)
+   ->  Custom Scan (SkipScan) on _hyper_4_10_chunk (actual rows=3 loops=1)
+         ->  Index Only Scan using _hyper_4_10_chunk_i3720_data_time_idx on _hyper_4_10_chunk (actual rows=3 loops=1)
+               Index Cond: (data > NULL::text)
+               Heap Fetches: 3
+(5 rows)
+

tsl/test/expected/plan_skip_scan-13.out

@@ -3941,3 +3941,26 @@ SELECT DISTINCT ON (a) * FROM i3629 WHERE a in (2) ORDER BY a ASC, time DESC;
  2 | Fri Mar 20 00:00:00 2020 PDT
 (1 row)
 
+-- #3720 skipscan not being used on varchar column
+CREATE TABLE i3720(time timestamptz not null,data varchar);
+SELECT table_name FROM create_hypertable('i3720','time');
+ table_name 
+------------
+ i3720
+(1 row)
+
+INSERT INTO i3720
+SELECT time, (array['Yes', 'No', 'Maybe'])[floor(random() * 3 + 1)]
+FROM generate_series('2000-01-01'::timestamptz,'2000-01-03'::timestamptz, '10 minute'::interval) AS g1(time);
+CREATE INDEX ON i3720(data, time);
+ANALYZE i3720;
+:PREFIX SELECT DISTINCT ON(data) * FROM i3720;
+                                                      QUERY PLAN                                                      
+----------------------------------------------------------------------------------------------------------------------
+ Unique (actual rows=3 loops=1)
+   ->  Custom Scan (SkipScan) on _hyper_4_10_chunk (actual rows=3 loops=1)
+         ->  Index Only Scan using _hyper_4_10_chunk_i3720_data_time_idx on _hyper_4_10_chunk (actual rows=3 loops=1)
+               Index Cond: (data > NULL::text)
+               Heap Fetches: 3
+(5 rows)
+

tsl/test/expected/plan_skip_scan-14.out

@@ -3939,3 +3939,26 @@ SELECT DISTINCT ON (a) * FROM i3629 WHERE a in (2) ORDER BY a ASC, time DESC;
  2 | Fri Mar 20 00:00:00 2020 PDT
 (1 row)
 
+-- #3720 skipscan not being used on varchar column
+CREATE TABLE i3720(time timestamptz not null,data varchar);
+SELECT table_name FROM create_hypertable('i3720','time');
+ table_name 
+------------
+ i3720
+(1 row)
+
+INSERT INTO i3720
+SELECT time, (array['Yes', 'No', 'Maybe'])[floor(random() * 3 + 1)]
+FROM generate_series('2000-01-01'::timestamptz,'2000-01-03'::timestamptz, '10 minute'::interval) AS g1(time);
+CREATE INDEX ON i3720(data, time);
+ANALYZE i3720;
+:PREFIX SELECT DISTINCT ON(data) * FROM i3720;
+                                                      QUERY PLAN                                                      
+----------------------------------------------------------------------------------------------------------------------
+ Unique (actual rows=3 loops=1)
+   ->  Custom Scan (SkipScan) on _hyper_4_10_chunk (actual rows=3 loops=1)
+         ->  Index Only Scan using _hyper_4_10_chunk_i3720_data_time_idx on _hyper_4_10_chunk (actual rows=3 loops=1)
+               Index Cond: (data > NULL::text)
+               Heap Fetches: 3
+(5 rows)
+

tsl/test/sql/plan_skip_scan.sql.in

@@ -26,3 +26,15 @@ INSERT INTO i3629 SELECT i, '2020-04-01'::date-10-i from generate_series(1,20) i
 EXPLAIN (SUMMARY OFF, COSTS OFF) SELECT DISTINCT ON (a) * FROM i3629 WHERE a in (2) ORDER BY a ASC, time DESC;
 SELECT DISTINCT ON (a) * FROM i3629 WHERE a in (2) ORDER BY a ASC, time DESC;
 
+-- #3720 skipscan not being used on varchar column
+CREATE TABLE i3720(time timestamptz not null,data varchar);
+SELECT table_name FROM create_hypertable('i3720','time');
+
+INSERT INTO i3720
+SELECT time, (array['Yes', 'No', 'Maybe'])[floor(random() * 3 + 1)]
+FROM generate_series('2000-01-01'::timestamptz,'2000-01-03'::timestamptz, '10 minute'::interval) AS g1(time);
+
+CREATE INDEX ON i3720(data, time);
+ANALYZE i3720;
+:PREFIX SELECT DISTINCT ON(data) * FROM i3720;
+