[
https://issues.apache.org/jira/browse/OAK-8918?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17063685#comment-17063685
]
Solomon Rutzky edited comment on OAK-8918 at 3/21/20, 3:36 AM:
---------------------------------------------------------------
Hi [~reschke]. I just looked at the code a little more closely and noticed a
minor issue with these changes. Assuming that the "collation_name" value on
this line:
[http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/rdb/RDBBlobStoreDB.java?revision=1874271&view=markup#l83]
comes from this line:
[http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/rdb/RDBCommonVendorSpecificCode.java?revision=1874174&view=markup#l96]
then the warning message on this line (i.e. "Default server collation is..."):
[http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/rdb/RDBBlobStoreDB.java?revision=1874271&view=markup#l85]
is not entirely correct. The collation being discovered in
RDBCommonVendorSpecificCode via {{sys.databases}} is the _database_ default
collation, not the server (or instance) default collation. While they are often
the same, there is no guarantee that they are the same. And to be clear, the
database collation is the more correct choice, so the warning message should be
updated to reflect that.
Now, to be even more pedantic, the {{ID}} column uses the database's default
collation because the {{COLLATE}} clause is not ( yet: OAK-8963 ) specified.
The assumption here is that the database's default collation has not changed
since the tables were created. Again, this is true most of the time, but cannot
be guaranteed. At the very least it should be noted that, in the current state
of the code (assuming the commits noted in this issue), it's at least
_possible_ that a false positive will result in the user being shown a warning
for a situation that is, in fact, _not_ a problem (e.g. if the DB's collation
is changed from a non-"SQL_" collation to a "SQL_" collation after the tables
were created).
*IN FACT*, now that I have explained the steps of that scenario I realize that
even in the case where the column's collation does match the database's default
collation, and that collation is a "{{SQL_*}}" collation, if the user gets the
warning message and makes the change recommended in OAK-8908 (i.e. changing the
collation of the two ID columns to {{Latin1_General_BIN}} ), then they will
_still_ receive this warning because the code isn't checking the collation of
the _columns_. Hence, it would seem that this issue has been implemented
incorrectly, and should instead be changed to:
{code:sql}
SELECT col.[collation_name]
FROM sys.columns col
WHERE col.[object_id] = OBJECT_ID(?)
AND col.[name] = N'ID';
{code}
where the "{{?}}" is set to "{{tableName}}". Just call it twice, once for each
table.
was (Author: solomon.rutzky):
Hi [~reschke]. I just looked at the code a little more closely and noticed a
minor issue with these changes. Assuming that the "collation_name" value on
this line:
[http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/rdb/RDBBlobStoreDB.java?revision=1874271&view=markup#l83]
comes from this line:
[http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/rdb/RDBCommonVendorSpecificCode.java?revision=1874174&view=markup#l96]
then the warning message on this line (i.e. "Default server collation is..."):
[http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/rdb/RDBBlobStoreDB.java?revision=1874271&view=markup#l85]
is not entirely correct. The collation being discovered in
RDBCommonVendorSpecificCode via {{sys.databases}} is the _database_ default
collation, not the server (or instance) default collation. While they are often
the same, there is no guarantee that they are the same. And to be clear, the
database collation is the more correct choice, so the warning message should be
updated to reflect that.
Now, to be even more pedantic, the {{ID}} column uses the database's default
collation because the {{COLLATE}} clause is not ( yet ) specified. The
assumption here is that the database's default collation has not changed since
the tables were created. Again, this is true most of the time, but cannot be
guaranteed. At the very least it should be noted that, in the current state of
the code (assuming the commits noted in this issue), it's at least _possible_
that a false positive will result in the user being shown a warning for a
situation that is, in fact, _not_ a problem (e.g. if the DB's collation is
changed from a non-"SQL_" collation to a "SQL_" collation after the tables were
created).
*IN FACT*, now that I have explained the steps of that scenario I realize that
even in the case where the column's collation does match the database's default
collation, and that collation is a "{{SQL_*}}" collation, if the user gets the
warning message and makes the change recommended in OAK-8908 (i.e. changing the
collation of the two ID columns to {{Latin1_General_BIN}} ), then they will
_still_ receive this warning because the code isn't checking the collation of
the _columns_. Hence, it would seem that this issue has been implemented
incorrectly, and should instead be changed to:
{code:sql}
SELECT col.[collation_name]
FROM sys.columns col
WHERE col.[object_id] = OBJECT_ID(?)
AND col.[name] = N'ID';
{code}
where the "{{?}}" is set to "{{tableName}}". Just call it twice, once for each
table.
> RDBBlobStore: warn when legacy (SQLServer) default collation is active
> ----------------------------------------------------------------------
>
> Key: OAK-8918
> URL: https://issues.apache.org/jira/browse/OAK-8918
> Project: Jackrabbit Oak
> Issue Type: Technical task
> Components: rdbmk
> Reporter: Julian Reschke
> Assignee: Julian Reschke
> Priority: Minor
> Labels: candidate_oak_1_8
> Fix For: 1.26.0, 1.10.9, 1.22.2
>
>
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
