Update QualifiedName to Store the Case-sensitive QualifiedName as well #359
Conversation
raveeram
commented
Aug 1, 2019
raveeram
commented
- Some Data-sources (like Cassandra) have case-sensitive database and table names. Preserve the casing so the corresponding Connector can use the right case-sensitive QualifiedName when needed.
raveeram
changed the title
| @@ -291,6 +298,47 @@ public static QualifiedName ofTable( | |||
| return new QualifiedName(catalogName, databaseName, tableName, null, null); | |||
| } | |||
|
|
|||
| private static QualifiedName createCaseSensitiveQualifiedNameIfNecessary( | |||
| @NonNull final String catalogName, | |||
There was a problem hiding this comment.
I don't have the full context, but I would create a separate function with the non-standardlized qualifiedname as the single input, then it returns the standard qualifiedName as one object. We can also create a new class as the standizedQualified name (which exends the current one) to handle the format and related operations.
There was a problem hiding this comment.
Agreed, but the expectation is that this is a one-off requirement and I wanted to avoid having to refactor all the QualifiedName constructors in the Controller/Service code to call a new API or a new QualifiedName type. This way, the change is contained inside the QualifiedName itself and the Connector makes a decision on whether to use the case-sensitive version or not.
| @@ -49,6 +49,9 @@ | |||
| private Map<String, String> parts; | |||
| private Type type; | |||
|
|
|||
| // Null if this QualifiedName was all lower-case to begin with. | |||
| private QualifiedName caseSensitiveQualifiedName; | |||
There was a problem hiding this comment.
What is the rationale of not using Optional?
There was a problem hiding this comment.
Optional is not serializable and also seems to be a general recommendation that Optional should be used in return values and not for class attributes: https://stackoverflow.com/questions/29033518/is-it-a-good-practice-to-use-optional-as-an-attribute-in-a-class
| @NonNull final String standardPartitionName, | ||
| @NonNull final String viewName, | ||
| @NonNull final String standardViewName) { | ||
| return (!databaseName.isEmpty() && !standardDatabaseName.equals(databaseName)) |
There was a problem hiding this comment.
Do we need the isEmpty check?
There was a problem hiding this comment.
Ah not anymore, was using the NonStandardized string earlier. Removed.
raveeram
force-pushed
the
AddOriginalQualifiedName
branch
from
0ddf1bf
to
e0b003f
Compare
raveeram
force-pushed
the
AddOriginalQualifiedName
branch
from
e0b003f
to
938126b
Compare
Netflix#359)" This reverts commit e5e69f5.
