On 31.12.2019 01:40, Peter Geoghegan wrote:
On Mon, Dec 30, 2019 at 9:45 AM Robert Haas <robertmh...@gmail.com> wrote:
For example, float and numeric types are "never bitwise equal", while array,
text, and other container types are "maybe bitwise equal". An array of
integers
or text with C collation can be treated as bitwise equal attributes, and it
would be too harsh to restrict them from deduplication.
We might as well support container types (like array) in the first
Postgres version that has nbtree deduplication, I suppose. Even still,
I don't think that it actually matters much to users. B-Tree indexes
on arrays are probably very rare. Note that I don't consider text to
be a container type here -- obviously btree/text_ops is a very
important opclass for the deduplication feature. It may be the most
important opclass overall.
Recursively invoking a support function for the "contained" data type
in the btree/array_ops support function seems like it might be messy.
Not sure about that, though.
What bothers me is that this option will unlikely be helpful on its own
and we
should also provide some kind of recheck function along with opclass, which
complicates this idea even further and doesn't seem very clear.
It seems like the simplest thing might be to forget about the 'char'
column and just have a support function which can be used to assess
whether a given opclass's notion of equality is bitwise.
I like the idea of relying only on a support function.
In attachment you can find the WIP patch that adds support function for
btree opclasses.
Before continuing, I want to ensure that I understood the discussion
above correctly.
Current version of the patch adds:
1) new syntax, which allow to provide support function:
CREATE OPERATOR CLASS int4_ops_test
FOR TYPE int4 USING btree AS
OPERATOR 1 =(int4, int4),
FUNCTION 1 btint4cmp(int4, int4),
SUPPORT datum_image_eqisbitwise;
We probably can add more words to specify the purpose of the support
function.
Do you have any other objections about the place of this new element in
CreateOplcass syntax structure?
2) trivial support function that always returns true
'datum_image_eqisbitwise'.
It is named after 'datum_image_eq', because we define this support
function via its behavior.
If this prototype is fine, I will continue this work and add support
functions for other opclasses, update pg_dump and documentation.
Thoughts?
commit 9eb37de922c699208e52349494653241ddeb0116
Author: anastasia <a.lubennik...@postgrespro.ru>
Date: Mon Jan 13 23:28:20 2020 +0300
v5-WIP-Opclass-bitwise-equality-0001.patch
diff --git a/src/backend/commands/opclasscmds.c b/src/backend/commands/opclasscmds.c
index e2c6de457c..143ecd1e13 100644
--- a/src/backend/commands/opclasscmds.c
+++ b/src/backend/commands/opclasscmds.c
@@ -335,7 +335,8 @@ DefineOpClass(CreateOpClassStmt *stmt)
storageoid, /* storage datatype oid, if any */
namespaceoid, /* namespace to create opclass in */
opfamilyoid, /* oid of containing opfamily */
- opclassoid; /* oid of opclass we create */
+ opclassoid, /* oid of opclass we create */
+ eqisbitwise_procoid; /* oid of opclass support function */
int maxOpNumber, /* amstrategies value */
maxProcNumber; /* amsupport value */
bool amstorage; /* amstorage flag */
@@ -564,6 +565,9 @@ DefineOpClass(CreateOpClassStmt *stmt)
aclcheck_error_type(ACLCHECK_NOT_OWNER, storageoid);
#endif
break;
+ case OPCLASS_ITEM_SUPPORT_FUNCTION:
+ eqisbitwise_procoid = LookupFuncName(item->name->objname, 0, false, false);
+ break;
default:
elog(ERROR, "unrecognized item type: %d", item->itemtype);
break;
@@ -653,6 +657,7 @@ DefineOpClass(CreateOpClassStmt *stmt)
values[Anum_pg_opclass_opcintype - 1] = ObjectIdGetDatum(typeoid);
values[Anum_pg_opclass_opcdefault - 1] = BoolGetDatum(stmt->isDefault);
values[Anum_pg_opclass_opckeytype - 1] = ObjectIdGetDatum(storageoid);
+ values[Anum_pg_opclass_opceqisbitwise - 1] = ObjectIdGetDatum(eqisbitwise_procoid);
tup = heap_form_tuple(rel->rd_att, values, nulls);
@@ -707,6 +712,15 @@ DefineOpClass(CreateOpClassStmt *stmt)
recordDependencyOn(&myself, &referenced, DEPENDENCY_NORMAL);
}
+ /* dependency on support function */
+ if (OidIsValid(eqisbitwise_procoid))
+ {
+ referenced.classId = ProcedureRelationId;
+ referenced.objectId = eqisbitwise_procoid;
+ referenced.objectSubId = 0;
+ recordDependencyOn(&myself, &referenced, DEPENDENCY_NORMAL);
+ }
+
/* dependency on owner */
recordDependencyOnOwner(OperatorClassRelationId, opclassoid, GetUserId());
diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
index 54ad62bb7f..b0cae4e0aa 100644
--- a/src/backend/nodes/copyfuncs.c
+++ b/src/backend/nodes/copyfuncs.c
@@ -3793,6 +3793,7 @@ _copyCreateOpClassStmt(const CreateOpClassStmt *from)
COPY_NODE_FIELD(datatype);
COPY_NODE_FIELD(items);
COPY_SCALAR_FIELD(isDefault);
+ COPY_STRING_FIELD(eqisbitwise_fn);
return newnode;
}
diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c
index 5b1ba143b1..eac66c6b84 100644
--- a/src/backend/nodes/equalfuncs.c
+++ b/src/backend/nodes/equalfuncs.c
@@ -1613,6 +1613,7 @@ _equalCreateOpClassStmt(const CreateOpClassStmt *a, const CreateOpClassStmt *b)
COMPARE_NODE_FIELD(datatype);
COMPARE_NODE_FIELD(items);
COMPARE_SCALAR_FIELD(isDefault);
+ COMPARE_STRING_FIELD(eqisbitwise_fn);
return true;
}
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index ad5be902b0..7fa9c0c14b 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -6019,6 +6019,13 @@ opclass_item:
n->storedtype = $2;
$$ = (Node *) n;
}
+ | SUPPORT function_with_argtypes
+ {
+ CreateOpClassItem *n = makeNode(CreateOpClassItem);
+ n->itemtype = OPCLASS_ITEM_SUPPORT_FUNCTION;
+ n->name = $2;
+ $$ = (Node *) n;
+ }
;
opt_default: DEFAULT { $$ = true; }
diff --git a/src/backend/utils/adt/datum.c b/src/backend/utils/adt/datum.c
index 4e81947352..f81037518f 100644
--- a/src/backend/utils/adt/datum.c
+++ b/src/backend/utils/adt/datum.c
@@ -323,6 +323,20 @@ datum_image_eq(Datum value1, Datum value2, bool typByVal, int typLen)
return result;
}
+/*-------------------------------------------------------------------------
+ * datum_image_eqisbitwise
+ *
+ * We define opclass safety for datum image comparison via the fact that
+ * opclass equality operator is always agree with datum_image_eq.
+ * So that is the trivial case.
+ *-------------------------------------------------------------------------
+ */
+Datum
+datum_image_eqisbitwise(PG_FUNCTION_ARGS)
+{
+ PG_RETURN_BOOL(true);
+}
+
/*-------------------------------------------------------------------------
* datumEstimateSpace
*
diff --git a/src/include/catalog/pg_opclass.dat b/src/include/catalog/pg_opclass.dat
index ab2f50c9eb..47d9f9da8c 100644
--- a/src/include/catalog/pg_opclass.dat
+++ b/src/include/catalog/pg_opclass.dat
@@ -21,11 +21,11 @@
{ opcmethod => 'hash', opcname => 'array_ops', opcfamily => 'hash/array_ops',
opcintype => 'anyarray' },
{ opcmethod => 'btree', opcname => 'bit_ops', opcfamily => 'btree/bit_ops',
- opcintype => 'bit' },
+ opcintype => 'bit' , opceqisbitwise => 'datum_image_eqisbitwise' },
{ opcmethod => 'btree', opcname => 'bool_ops', opcfamily => 'btree/bool_ops',
- opcintype => 'bool' },
+ opcintype => 'bool' , opceqisbitwise => 'datum_image_eqisbitwise'},
{ opcmethod => 'btree', opcname => 'bpchar_ops',
- opcfamily => 'btree/bpchar_ops', opcintype => 'bpchar' },
+ opcfamily => 'btree/bpchar_ops', opcintype => 'bpchar'},
{ opcmethod => 'hash', opcname => 'bpchar_ops', opcfamily => 'hash/bpchar_ops',
opcintype => 'bpchar' },
{ opcmethod => 'btree', opcname => 'bytea_ops', opcfamily => 'btree/bytea_ops',
@@ -62,17 +62,17 @@
opcfamily => 'spgist/network_ops', opcintype => 'inet' },
{ oid => '1979', oid_symbol => 'INT2_BTREE_OPS_OID',
opcmethod => 'btree', opcname => 'int2_ops', opcfamily => 'btree/integer_ops',
- opcintype => 'int2' },
+ opcintype => 'int2', opceqisbitwise => 'datum_image_eqisbitwise' },
{ opcmethod => 'hash', opcname => 'int2_ops', opcfamily => 'hash/integer_ops',
opcintype => 'int2' },
{ oid => '1978', oid_symbol => 'INT4_BTREE_OPS_OID',
opcmethod => 'btree', opcname => 'int4_ops', opcfamily => 'btree/integer_ops',
- opcintype => 'int4' },
+ opcintype => 'int4', opceqisbitwise => 'datum_image_eqisbitwise' },
{ opcmethod => 'hash', opcname => 'int4_ops', opcfamily => 'hash/integer_ops',
opcintype => 'int4' },
{ oid => '3124', oid_symbol => 'INT8_BTREE_OPS_OID',
opcmethod => 'btree', opcname => 'int8_ops', opcfamily => 'btree/integer_ops',
- opcintype => 'int8' },
+ opcintype => 'int8', opceqisbitwise => 'datum_image_eqisbitwise' },
{ opcmethod => 'hash', opcname => 'int8_ops', opcfamily => 'hash/integer_ops',
opcintype => 'int8' },
{ opcmethod => 'btree', opcname => 'interval_ops',
diff --git a/src/include/catalog/pg_opclass.h b/src/include/catalog/pg_opclass.h
index 963ab3eae8..304fc59f1e 100644
--- a/src/include/catalog/pg_opclass.h
+++ b/src/include/catalog/pg_opclass.h
@@ -73,6 +73,10 @@ CATALOG(pg_opclass,2616,OperatorClassRelationId)
/* type of data in index, or InvalidOid */
Oid opckeytype BKI_DEFAULT(0) BKI_LOOKUP(pg_type);
+
+ /* support function (0 if none)*/
+ regproc opceqisbitwise BKI_DEFAULT(-) BKI_LOOKUP(pg_proc);
+
} FormData_pg_opclass;
/* ----------------
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 427faa3c3b..4fc8aa20b5 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -10745,4 +10745,10 @@
proname => 'pg_partition_root', prorettype => 'regclass',
proargtypes => 'regclass', prosrc => 'pg_partition_root' },
+ # support functions for operator classes
+ # trivial cases use datum_image_eqisbitwise as source function
+{ oid => '560',
+ proname => 'datum_image_eqisbitwise', proleakproof => 't', prorettype => 'bool',
+ proargtypes => '', prosrc => 'datum_image_eqisbitwise' },
+
]
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index cdfa0568f7..8838250a92 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -2598,11 +2598,14 @@ typedef struct CreateOpClassStmt
TypeName *datatype; /* datatype of indexed column */
List *items; /* List of CreateOpClassItem nodes */
bool isDefault; /* Should be marked as default for type? */
+ char *eqisbitwise_fn; /* name of support function to check if opclass
+ * equality operator provides bitwise comparator */
} CreateOpClassStmt;
#define OPCLASS_ITEM_OPERATOR 1
#define OPCLASS_ITEM_FUNCTION 2
#define OPCLASS_ITEM_STORAGETYPE 3
+#define OPCLASS_ITEM_SUPPORT_FUNCTION 4
typedef struct CreateOpClassItem
{
diff --git a/src/test/regress/expected/opr_sanity.out b/src/test/regress/expected/opr_sanity.out
index c19740e5db..263cc5545b 100644
--- a/src/test/regress/expected/opr_sanity.out
+++ b/src/test/regress/expected/opr_sanity.out
@@ -468,9 +468,10 @@ SELECT p1.oid, p1.proname
FROM pg_proc as p1 LEFT JOIN pg_description as d
ON p1.tableoid = d.classoid and p1.oid = d.objoid and d.objsubid = 0
WHERE d.classoid IS NULL AND p1.oid <= 9999;
- oid | proname
------+---------
-(0 rows)
+ oid | proname
+-----+-------------------------
+ 560 | datum_image_eqisbitwise
+(1 row)
-- List of built-in leakproof functions
--
@@ -587,6 +588,7 @@ int84lt(bigint,integer)
int84gt(bigint,integer)
int84le(bigint,integer)
int84ge(bigint,integer)
+datum_image_eqisbitwise()
oidvectorne(oidvector,oidvector)
namelt(name,name)
namele(name,name)
