Skip to content

Check actual hash support when altering a distribution key type - #2308

Merged
reshke merged 3 commits into
apache:mainfrom
reshke:fix-mpp-alter-distribution-hashability-20261112
Nov 14, 2026
Merged

reshke merged 3 commits into
apache:mainfrom
reshke:fix-mpp-alter-distribution-hashability-20261112

Conversation

@reshke

@reshke reshke commented Nov 12, 2026

Copy link
Copy Markdown
Contributor

ALTER COLUMN TYPE looked up a default hash operator class without checking whether its support functions can hash the concrete element or field types. For example, this left an empty table distributed by money[]:

CREATE TABLE t (k int[]) DISTRIBUTED BY (k);
ALTER TABLE t ALTER COLUMN k TYPE money[] USING k::money[];
INSERT INTO t VALUES (ARRAY[1::money]);
ERROR: could not identify a hash function for type money

Both array types select array_ops, so ATPrepAlterColumnType considered their distribution compatible even though money has no default hash support. Use cdb_default_distribution_opclass_for_type, as CREATE TABLE does, so the existing incompatible-type handling also covers containers: reject the change on a populated table, or switch an empty table to random distribution.

Add coverage for both cases and for a compatible int[] to bigint[] change.

Fixes #ISSUE_Number

What does this PR do?

Type of Change

  • Bug fix (non-breaking change)
  • New feature (non-breaking change)
  • Breaking change (fix or feature with breaking changes)
  • Documentation update

Breaking Changes

Test Plan

  • Unit tests added/updated
  • Integration tests added/updated
  • Passed make installcheck
  • Passed make -C src/test installcheck-cbdb-parallel

Impact

Performance:

User-facing changes:

Dependencies:

Checklist

Additional Context

CI Skip Instructions


@my-ship-it my-ship-it left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM ,thanks

@my-ship-it

Copy link
Copy Markdown
Contributor

Some cases not covered:

case 1

SET gp_use_legacy_hashops = on;
CREATE TABLE lm (m money, x int) DISTRIBUTED BY (m); 
CREATE TABLE c AS SELECT * FROM lm; 
ERROR:  cache lookup failed for opclass 0 (lsyscache.c:1365)

case 2

CREATE TYPE ct AS (a int);
CREATE TABLE tc (c ct, x int) DISTRIBUTED BY (c);
ALTER TYPE ct ADD ATTRIBUTE m money; 
INSERT INTO tc VALUES (ROW(2, 2::money)::ct, 2);
ERROR:  could not identify a hash function for type money
SELECT count(*) FROM tc WHERE c = ROW(1,NULL)::ct;
ERROR:  could not identify a hash function for type money

@reshke

reshke commented Nov 12, 2026

Copy link
Copy Markdown
Contributor Author

Some cases not covered:

case 1

SET gp_use_legacy_hashops = on;
CREATE TABLE lm (m money, x int) DISTRIBUTED BY (m); 
CREATE TABLE c AS SELECT * FROM lm; 
ERROR:  cache lookup failed for opclass 0 (lsyscache.c:1365)

case 2

CREATE TYPE ct AS (a int);
CREATE TABLE tc (c ct, x int) DISTRIBUTED BY (c);
ALTER TYPE ct ADD ATTRIBUTE m money; 
INSERT INTO tc VALUES (ROW(2, 2::money)::ct, 2);
ERROR:  could not identify a hash function for type money
SELECT count(*) FROM tc WHERE c = ROW(1,NULL)::ct;
ERROR:  could not identify a hash function for type money

Yes, thx, this report looks related, will fix inside this PR

reshke added a commit to reshke/cloudberrydb that referenced this pull request Nov 12, 2026
Check concrete hash support when ALTER COLUMN TYPE or CTAS chooses a
new distribution key. A generic array or record operator class alone does
not guarantee that its element or field types can be hashed. Retain the
existing ALTER COLUMN behavior: reject an incompatible change on populated
tables, or use random distribution for empty tables. CTAS skips unusable
default classes so the fallback can select a legacy class, another key,
or random distribution.

The affected paths previously produced:

    ALTER COLUMN k TYPE money[] followed by INSERT:
    ERROR: could not identify a hash function for type money

    CTAS from a legacy-distributed money key with the PostgreSQL planner:
    ERROR: cache lookup failed for opclass 0 (lsyscache.c:1366)

Also reject adding or dropping row-type attributes while the type is used
in a distribution key, directly or through a container. Even adding a
hashable NULL field changes hash_record: in a two-segment check, 13 of 20
existing values then required a different segment. Protect table row types
and cascading attribute drops too. Hold locks on traversed types and
referencing relations to serialize new dependencies and policy changes;
ignore expression-only dependencies such as domain defaults.

Shape changes remain available for non-key columns and random or replicated
tables. Dependent empty hash-distributed tables are deliberately guarded as
well; change their distribution policy before changing the key's row type.

Add coverage for ALTER COLUMN, legacy CTAS, nested row types, cascades and
allowed shape changes. Adapt existing row-value deformation fixtures to use
another key or random distribution so they exercise their original purpose.

Includes the follow-ups reported in apache#2308,
issuecomment-6819847655.
@reshke
reshke force-pushed the fix-mpp-alter-distribution-hashability-20261112 branch from c8983b9 to e3f6cd6 Compare November 12, 2026 12:44
reshke added a commit to reshke/cloudberrydb that referenced this pull request Nov 13, 2026
Check concrete hash support when ALTER COLUMN TYPE or CTAS chooses a
new distribution key. A generic array or record operator class alone does
not guarantee that its element or field types can be hashed. Retain the
existing ALTER COLUMN behavior: reject an incompatible change on populated
tables, or use random distribution for empty tables. CTAS skips unusable
default classes so the fallback can select a legacy class, another key,
or random distribution.

The affected paths previously produced:

    ALTER COLUMN k TYPE money[] followed by INSERT:
    ERROR: could not identify a hash function for type money

    CTAS from a legacy-distributed money key with the PostgreSQL planner:
    ERROR: cache lookup failed for opclass 0 (lsyscache.c:1366)

Also reject adding or dropping row-type attributes while the type is used
in a distribution key, directly or through a container. Even adding a
hashable NULL field changes hash_record: in a two-segment check, 13 of 20
existing values then required a different segment. Protect table row types
and cascading attribute drops too. Hold locks on traversed types and
referencing relations to serialize new dependencies and policy changes;
ignore expression-only dependencies such as domain defaults.

Shape changes remain available for non-key columns and random or replicated
tables. Dependent empty hash-distributed tables are deliberately guarded as
well; change their distribution policy before changing the key's row type.

Add coverage for ALTER COLUMN, legacy CTAS, nested row types, cascades and
allowed shape changes. Adapt existing row-value deformation fixtures to use
another key or random distribution so they exercise their original purpose.

Includes the follow-ups reported in apache#2308,
issuecomment-6819847655.
@reshke
reshke force-pushed the fix-mpp-alter-distribution-hashability-20261112 branch from e2189ca to af78cb8 Compare November 13, 2026 17:37
reshke added a commit to reshke/cloudberrydb that referenced this pull request Nov 14, 2026
Check concrete hash support when ALTER COLUMN TYPE or CTAS chooses a
new distribution key. A generic array or record operator class alone does
not guarantee that its element or field types can be hashed. Retain the
existing ALTER COLUMN behavior: reject an incompatible change on populated
tables, or use random distribution for empty tables. CTAS skips unusable
default classes so the fallback can select a legacy class, another key,
or random distribution.

The affected paths previously produced:

    ALTER COLUMN k TYPE money[] followed by INSERT:
    ERROR: could not identify a hash function for type money

    CTAS from a legacy-distributed money key with the PostgreSQL planner:
    ERROR: cache lookup failed for opclass 0 (lsyscache.c:1366)

Also reject adding or dropping row-type attributes while the type is used
in a distribution key, directly or through a container. Even adding a
hashable NULL field changes hash_record: in a two-segment check, 13 of 20
existing values then required a different segment. Protect table row types
and cascading attribute drops too. Hold locks on traversed types and
referencing relations to serialize new dependencies and policy changes;
ignore expression-only dependencies such as domain defaults.

Shape changes remain available for non-key columns and random or replicated
tables. Dependent empty hash-distributed tables are deliberately guarded as
well; change their distribution policy before changing the key's row type.

Add coverage for ALTER COLUMN, legacy CTAS, nested row types, cascades and
allowed shape changes. Adapt existing row-value deformation fixtures to use
another key or random distribution so they exercise their original purpose.

Includes the follow-ups reported in apache#2308,
issuecomment-6819847655.
@reshke
reshke force-pushed the fix-mpp-alter-distribution-hashability-20261112 branch from af78cb8 to 5e8d554 Compare November 14, 2026 02:31
reshke added a commit to reshke/cloudberrydb that referenced this pull request Nov 14, 2026
Check concrete hash support when ALTER COLUMN TYPE or CTAS chooses a
new distribution key. A generic array or record operator class alone does
not guarantee that its element or field types can be hashed. Retain the
existing ALTER COLUMN behavior: reject an incompatible change on populated
tables, or use random distribution for empty tables. CTAS skips unusable
default classes so the fallback can select a legacy class, another key,
or random distribution.

The affected paths previously produced:

    ALTER COLUMN k TYPE money[] followed by INSERT:
    ERROR: could not identify a hash function for type money

    CTAS from a legacy-distributed money key with the PostgreSQL planner:
    ERROR: cache lookup failed for opclass 0 (lsyscache.c:1366)

Also reject adding or dropping row-type attributes while the type is used
in a distribution key, directly or through a container. Even adding a
hashable NULL field changes hash_record: in a two-segment check, 13 of 20
existing values then required a different segment. Protect table row types
and cascading attribute drops too. Hold locks on traversed types and
referencing relations to serialize new dependencies and policy changes;
ignore expression-only dependencies such as domain defaults.

Shape changes remain available for non-key columns and random or replicated
tables. Dependent empty hash-distributed tables are deliberately guarded as
well; change their distribution policy before changing the key's row type.

Add coverage for ALTER COLUMN, legacy CTAS, nested row types, cascades and
allowed shape changes. Adapt existing row-value deformation fixtures to use
another key or random distribution so they exercise their original purpose.

Includes the follow-ups reported in apache#2308,
issuecomment-6819847655.
@reshke
reshke force-pushed the fix-mpp-alter-distribution-hashability-20261112 branch from 5e8d554 to 96bf8f2 Compare November 14, 2026 06:19
@reshke
reshke enabled auto-merge (rebase) November 14, 2026 14:51
Check concrete hash support when ALTER COLUMN TYPE or CTAS chooses a
new distribution key. A generic array or record operator class alone does
not guarantee that its element or field types can be hashed. Retain the
existing ALTER COLUMN behavior: reject an incompatible change on populated
tables, or use random distribution for empty tables. CTAS skips unusable
default classes so the fallback can select a legacy class, another key,
or random distribution.

The affected paths previously produced:

    ALTER COLUMN k TYPE money[] followed by INSERT:
    ERROR: could not identify a hash function for type money

    CTAS from a legacy-distributed money key with the PostgreSQL planner:
    ERROR: cache lookup failed for opclass 0 (lsyscache.c:1366)

Also reject adding or dropping row-type attributes while the type is used
in a distribution key, directly or through a container. Even adding a
hashable NULL field changes hash_record: in a two-segment check, 13 of 20
existing values then required a different segment. Protect table row types
and cascading attribute drops too. Hold locks on traversed types and
referencing relations to serialize new dependencies and policy changes;
ignore expression-only dependencies such as domain defaults.

Shape changes remain available for non-key columns and random or replicated
tables. Dependent empty hash-distributed tables are deliberately guarded as
well; change their distribution policy before changing the key's row type.

Add coverage for ALTER COLUMN, legacy CTAS, nested row types, cascades and
allowed shape changes. Adapt existing row-value deformation fixtures to use
another key or random distribution so they exercise their original purpose.

Includes the follow-ups reported in apache#2308,
issuecomment-6819847655.
Regression output has no blank line before the added test cases:

  reset optimizer_trace_fallback;
  -- A source distribution key need not have a usable default hash opclass.

Remove the extra lines from both gpctas expected variants and alter_distribution_policy.
Keep the mutable fullname rowtype out of the distribution key in the PAX copy of rowtypes. In singlenode mode distribution clauses have no effect and emit a warning, so retain the original CTAS fixture there.
@reshke
reshke force-pushed the fix-mpp-alter-distribution-hashability-20261112 branch from 96bf8f2 to 8fd2049 Compare November 14, 2026 14:51
@reshke
reshke disabled auto-merge November 14, 2026 14:51
@reshke
reshke enabled auto-merge (squash) November 14, 2026 14:51
@reshke
reshke merged commit 29929ea into apache:main Nov 14, 2026
123 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants