Skip to content

Conversation

@dyemanov
Copy link
Member

This is a postfix for PR #7748. Before this PR, if the field's collation was derived from some domain, RDB$COLLATION_ID was not filled inside RDB$RELATION_FIELDS and collation was inherited from the domain definition. Now it's broken.

Test case:

CREATE DOMAIN d1 VARCHAR(30) CHARACTER SET UTF8 COLLATE UNICODE;
CREATE TABLE test(data1 d1);
COMMIT;
INSERT INTO test VALUES('A');
INSERT INTO test VALUES('a');
COMMIT;

ALTER DOMAIN d1 TYPE VARCHAR(30) CHARACTER SET UTF8 COLLATE UNICODE_CI;
COMMIT;

SELECT data1 FROM test WHERE data1 = 'a';
DATA1                          
============================== 
a

Expected:

DATA1                          
============================== 
A                              
a

@dyemanov dyemanov self-assigned this Feb 11, 2025
@aafemt
Copy link
Contributor

aafemt commented Feb 11, 2025

IIRC this change can break the case when field is defined with domain and explicit collation. Most likely both conditions must be checked here.

@dyemanov
Copy link
Member Author

IIRC this change can break the case when field is defined with domain and explicit collation. Most likely both conditions must be checked here.

True. But this seems to be against the SQL standard:

11.4 "column definition"
12) Case:
a) If "column definition" immediately contains "domain name", then it shall not also immediately contain "collate clause".

So perhaps this should be disallowed rather than fixed.

@aafemt
Copy link
Contributor

aafemt commented Feb 11, 2025

I see no rationale behind this piece of standard but the decision is on you.

BTW, changing of collation requires index rebuild. If you change collation of domain, will every index on every domain-based field be rebuilt automatically?

@dyemanov
Copy link
Member Author

IIRC, yes, indices are rebuilt.

@dyemanov
Copy link
Member Author

I see no rationale behind this piece of standard but the decision is on you.

Personally, I don't see much value in this feature, but it seems it was supported before your PR, so this becomes a backward compatibility issue.

@aafemt
Copy link
Contributor

aafemt commented Feb 11, 2025

In this case IMHO it must be a rule at the parser level that COLLATE clause cannot be used with domains.

@dyemanov
Copy link
Member Author

As such a change wasn't suggested in the original discussion (although mentioned as deviation from the standard), I will not change it for the time being.

@dyemanov dyemanov linked an issue Feb 12, 2025 that may be closed by this pull request
@dyemanov dyemanov merged commit 120d2f6 into master Feb 12, 2025
46 of 48 checks passed
@dyemanov dyemanov deleted the work/7748-domain-collate-postfix branch February 12, 2025 13:40
@pavel-zotov
Copy link

pavel-zotov commented Feb 12, 2025

Currently only UTF8 charset is checked - see notes in the test file.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

"alter domain" supporting collation change [CORE1202]

3 participants