You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@phoenix.apache.org by GitBox <gi...@apache.org> on 2021/07/26 14:38:54 UTC
[GitHub] [phoenix] richardantal opened a new pull request #1275: PHOENIX-6519 Make SchemaTool work with lower case table and column names
richardantal opened a new pull request #1275:
URL: https://github.com/apache/phoenix/pull/1275
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@phoenix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] stoty commented on pull request #1275: PHOENIX-6519 Make SchemaTool work with lower case table and column names
Posted by GitBox <gi...@apache.org>.
stoty commented on pull request #1275:
URL: https://github.com/apache/phoenix/pull/1275#issuecomment-886909512
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 12s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | No case conflicting files found. |
| +1 :green_heart: | hbaseanti | 0m 0s | Patch does not have any anti-patterns. |
| +1 :green_heart: | @author | 0m 0s | The patch does not contain any @author tags. |
| -1 :x: | test4tests | 0m 0s | The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. |
||| _ master Compile Tests _ |
| +0 :ok: | mvndep | 5m 12s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 20m 23s | master passed |
| +0 | hbaserecompile | 32m 34s | HBase recompiled. |
| +1 :green_heart: | compile | 1m 39s | master passed |
| +1 :green_heart: | checkstyle | 0m 56s | master passed |
| +1 :green_heart: | javadoc | 1m 13s | master passed |
| +0 :ok: | spotbugs | 3m 40s | phoenix-core in master has 964 extant spotbugs warnings. |
| +0 :ok: | spotbugs | 0m 47s | phoenix-tools in master has 5 extant spotbugs warnings. |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 12s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 16m 59s | the patch passed |
| +0 | hbaserecompile | 24m 50s | HBase recompiled. |
| +1 :green_heart: | compile | 1m 36s | the patch passed |
| +1 :green_heart: | javac | 1m 36s | the patch passed |
| -1 :x: | checkstyle | 0m 44s | phoenix-core: The patch generated 33 new + 712 unchanged - 0 fixed = 745 total (was 712) |
| -1 :x: | checkstyle | 0m 16s | phoenix-tools: The patch generated 15 new + 213 unchanged - 12 fixed = 228 total (was 225) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | javadoc | 1m 6s | the patch passed |
| +1 :green_heart: | spotbugs | 4m 25s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 119m 10s | phoenix-core in the patch passed. |
| -1 :x: | unit | 3m 30s | phoenix-tools in the patch failed. |
| +1 :green_heart: | asflicense | 0m 24s | The patch does not generate ASF License warnings. |
| | | 199m 48s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1275/1/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/phoenix/pull/1275 |
| Optional Tests | dupname asflicense javac javadoc unit spotbugs hbaserebuild hbaseanti checkstyle compile |
| uname | Linux 015f7b580e53 4.15.0-147-generic #151-Ubuntu SMP Fri Jun 18 19:21:19 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev/phoenix-personality.sh |
| git revision | master / 6842c1d |
| Default Java | Private Build-1.8.0_242-8u242-b08-0ubuntu3~16.04-b08 |
| checkstyle | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1275/1/artifact/yetus-general-check/output/diff-checkstyle-phoenix-core.txt |
| checkstyle | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1275/1/artifact/yetus-general-check/output/diff-checkstyle-phoenix-tools.txt |
| unit | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1275/1/artifact/yetus-general-check/output/patch-unit-phoenix-tools.txt |
| Test Results | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1275/1/testReport/ |
| Max. process+thread count | 11543 (vs. ulimit of 30000) |
| modules | C: phoenix-core phoenix-tools U: . |
| Console output | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1275/1/console |
| versions | git=2.7.4 maven=3.3.9 spotbugs=4.1.3 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@phoenix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] stoty commented on a change in pull request #1275: PHOENIX-6519 Make SchemaTool work with lower case table and column names
Posted by GitBox <gi...@apache.org>.
stoty commented on a change in pull request #1275:
URL: https://github.com/apache/phoenix/pull/1275#discussion_r678999374
##########
File path: phoenix-core/src/main/java/org/apache/phoenix/util/SchemaUtil.java
##########
@@ -1238,6 +1238,78 @@ public static String getNormalizedColumnName(ColumnParseNode columnParseNode) {
return columnParseNode.getName();
}
+ public static String getFullTableNameWithQuotes(String schemaName, String tableName,
+ boolean schemaNameCaseSensitive, boolean tableNameCaseSensitive) {
+ String fullTableName;
+
+ if (tableNameCaseSensitive) {
+ fullTableName = "\"" + tableName + "\"";
+ } else {
+ fullTableName = tableName;
+ }
+
+ if(schemaName != null && schemaName.length() != 0) {
+ if (schemaNameCaseSensitive) {
+ fullTableName = "\"" + schemaName + "\"" + QueryConstants.NAME_SEPARATOR + fullTableName;
+ } else {
+ fullTableName = schemaName + QueryConstants.NAME_SEPARATOR + fullTableName;
+ }
+ }
+ return fullTableName;
+ }
+
+ public static String getFullTableNameWithQuotes(String schemaName, String tableName) {
+ return getFullTableNameWithQuotes(schemaName, tableName,
+ quotesNeededForSchema(schemaName), quotesNeededForTable(tableName));
+ }
+
+ private static boolean quotesNeededForSchema(String name) {
+ if (Strings.isNullOrEmpty(name) || name.equals(QueryConstants.DEFAULT_COLUMN_FAMILY)) {
+ return false;
+ }
+ return quotesNeededForTable(name);
+ }
+
+ private static boolean quotesNeededForColumn(String name) {
+ if (!name.equals("_INDEX_ID") && "_".equals(String.valueOf(name.charAt(0)))) {
Review comment:
Thanks for the explanation.
`name.startsWith("_")`
would be a far simpler check. (In the other methods as well.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@phoenix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] richardantal merged pull request #1275: PHOENIX-6519 Make SchemaTool work with lower case table and column names
Posted by GitBox <gi...@apache.org>.
richardantal merged pull request #1275:
URL: https://github.com/apache/phoenix/pull/1275
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@phoenix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] stoty commented on a change in pull request #1275: PHOENIX-6519 Make SchemaTool work with lower case table and column names
Posted by GitBox <gi...@apache.org>.
stoty commented on a change in pull request #1275:
URL: https://github.com/apache/phoenix/pull/1275#discussion_r679111687
##########
File path: phoenix-core/src/main/java/org/apache/phoenix/util/SchemaUtil.java
##########
@@ -1238,6 +1238,78 @@ public static String getNormalizedColumnName(ColumnParseNode columnParseNode) {
return columnParseNode.getName();
}
+ public static String getFullTableNameWithQuotes(String schemaName, String tableName,
+ boolean schemaNameCaseSensitive, boolean tableNameCaseSensitive) {
+ String fullTableName;
+
+ if (tableNameCaseSensitive) {
+ fullTableName = "\"" + tableName + "\"";
+ } else {
+ fullTableName = tableName;
+ }
+
+ if(schemaName != null && schemaName.length() != 0) {
+ if (schemaNameCaseSensitive) {
+ fullTableName = "\"" + schemaName + "\"" + QueryConstants.NAME_SEPARATOR + fullTableName;
+ } else {
+ fullTableName = schemaName + QueryConstants.NAME_SEPARATOR + fullTableName;
+ }
+ }
+ return fullTableName;
+ }
+
+ public static String getFullTableNameWithQuotes(String schemaName, String tableName) {
+ return getFullTableNameWithQuotes(schemaName, tableName,
+ quotesNeededForSchema(schemaName), quotesNeededForTable(tableName));
+ }
+
+ private static boolean quotesNeededForSchema(String name) {
+ if (Strings.isNullOrEmpty(name) || name.equals(QueryConstants.DEFAULT_COLUMN_FAMILY)) {
+ return false;
+ }
+ return quotesNeededForTable(name);
+ }
+
+ private static boolean quotesNeededForColumn(String name) {
+ if (!name.equals("_INDEX_ID") && name.startsWith("_")) {
+ return true;
+ }
+ return isQuotesNeeded(name) || containsLowerCase(name);
+ }
+
+ private static boolean quotesNeededForTable(String name) {
+ if (name.startsWith("_")) {
+ return true;
+ }
+ return isQuotesNeeded(name) || containsLowerCase(name);
+ }
+
+ public static String formatSchemaName(String name) {
+ if (quotesNeededForSchema(name)) {
Review comment:
Does the leading _ issue not apply here ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@phoenix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] richardantal commented on pull request #1275: PHOENIX-6519 Make SchemaTool work with lower case table and column names
Posted by GitBox <gi...@apache.org>.
richardantal commented on pull request #1275:
URL: https://github.com/apache/phoenix/pull/1275#issuecomment-889143359
Thank you @stoty for the reviews!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@phoenix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] stoty commented on a change in pull request #1275: PHOENIX-6519 Make SchemaTool work with lower case table and column names
Posted by GitBox <gi...@apache.org>.
stoty commented on a change in pull request #1275:
URL: https://github.com/apache/phoenix/pull/1275#discussion_r678881263
##########
File path: phoenix-core/src/main/java/org/apache/phoenix/util/SchemaUtil.java
##########
@@ -1238,6 +1238,78 @@ public static String getNormalizedColumnName(ColumnParseNode columnParseNode) {
return columnParseNode.getName();
}
+ public static String getFullTableNameWithQuotes(String schemaName, String tableName,
+ boolean schemaNameCaseSensitive, boolean tableNameCaseSensitive) {
+ String fullTableName;
+
+ if (tableNameCaseSensitive) {
+ fullTableName = "\"" + tableName + "\"";
+ } else {
+ fullTableName = tableName;
+ }
+
+ if(schemaName != null && schemaName.length() != 0) {
+ if (schemaNameCaseSensitive) {
+ fullTableName = "\"" + schemaName + "\"" + QueryConstants.NAME_SEPARATOR + fullTableName;
+ } else {
+ fullTableName = schemaName + QueryConstants.NAME_SEPARATOR + fullTableName;
+ }
+ }
+ return fullTableName;
+ }
+
+ public static String getFullTableNameWithQuotes(String schemaName, String tableName) {
+ return getFullTableNameWithQuotes(schemaName, tableName,
+ quotesNeededForSchema(schemaName), quotesNeededForTable(tableName));
+ }
+
+ private static boolean quotesNeededForSchema(String name) {
+ if (Strings.isNullOrEmpty(name) || name.equals(QueryConstants.DEFAULT_COLUMN_FAMILY)) {
+ return false;
+ }
+ return quotesNeededForTable(name);
+ }
+
+ private static boolean quotesNeededForColumn(String name) {
+ if (!name.equals("_INDEX_ID") && "_".equals(String.valueOf(name.charAt(0)))) {
Review comment:
Why do we need the special case for tables beginning with underscore ?
Also getting the first character as string would can probably done in a less circumpect way.
##########
File path: phoenix-tools/src/main/java/org/apache/phoenix/schema/SchemaExtractionProcessor.java
##########
@@ -249,7 +261,7 @@ public String extractCreateTableDDL(PTable table) throws IOException, SQLExcepti
private String generateTableDDLString(String columnInfoString, String propertiesString,
String pSchemaName, String pTableName) {
- String pTableFullName = SchemaUtil.getPTableFullNameWithQuotes(pSchemaName, pTableName);
+ String pTableFullName = SchemaUtil.getFullTableNameWithQuotes(pSchemaName, pTableName);
Review comment:
You may want to go over the code, and change the quoted variable names according to some common template, to improbe readability.
##########
File path: phoenix-tools/src/it/java/org/apache/phoenix/schema/SchemaToolExtractionIT.java
##########
@@ -91,6 +104,42 @@ public void testCreateIndexStatement() throws Exception {
Assert.assertEquals(createIndexStatement2.toUpperCase(), result.toUpperCase());
}
+ @Test
+ public void testCreateLocalIndexStatement() throws Exception {
Review comment:
It seems that you're also fixng some bugs WRT local indexes.
You should probablay note it at least in the commit message.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@phoenix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] richardantal commented on a change in pull request #1275: PHOENIX-6519 Make SchemaTool work with lower case table and column names
Posted by GitBox <gi...@apache.org>.
richardantal commented on a change in pull request #1275:
URL: https://github.com/apache/phoenix/pull/1275#discussion_r678984841
##########
File path: phoenix-core/src/main/java/org/apache/phoenix/util/SchemaUtil.java
##########
@@ -1238,6 +1238,78 @@ public static String getNormalizedColumnName(ColumnParseNode columnParseNode) {
return columnParseNode.getName();
}
+ public static String getFullTableNameWithQuotes(String schemaName, String tableName,
+ boolean schemaNameCaseSensitive, boolean tableNameCaseSensitive) {
+ String fullTableName;
+
+ if (tableNameCaseSensitive) {
+ fullTableName = "\"" + tableName + "\"";
+ } else {
+ fullTableName = tableName;
+ }
+
+ if(schemaName != null && schemaName.length() != 0) {
+ if (schemaNameCaseSensitive) {
+ fullTableName = "\"" + schemaName + "\"" + QueryConstants.NAME_SEPARATOR + fullTableName;
+ } else {
+ fullTableName = schemaName + QueryConstants.NAME_SEPARATOR + fullTableName;
+ }
+ }
+ return fullTableName;
+ }
+
+ public static String getFullTableNameWithQuotes(String schemaName, String tableName) {
+ return getFullTableNameWithQuotes(schemaName, tableName,
+ quotesNeededForSchema(schemaName), quotesNeededForTable(tableName));
+ }
+
+ private static boolean quotesNeededForSchema(String name) {
+ if (Strings.isNullOrEmpty(name) || name.equals(QueryConstants.DEFAULT_COLUMN_FAMILY)) {
+ return false;
+ }
+ return quotesNeededForTable(name);
+ }
+
+ private static boolean quotesNeededForColumn(String name) {
+ if (!name.equals("_INDEX_ID") && "_".equals(String.valueOf(name.charAt(0)))) {
Review comment:
If we want to create a table named _FOO we need quotes around that. The same applies for column names.
The difference is that when we create an index, _INDEX_ID column is added automatically and we do not want to have quotes around that. (But it should be possible to create a table called "_INDEX_ID" even if it an abnormal name.)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@phoenix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] stoty commented on pull request #1275: PHOENIX-6519 Make SchemaTool work with lower case table and column names
Posted by GitBox <gi...@apache.org>.
stoty commented on pull request #1275:
URL: https://github.com/apache/phoenix/pull/1275#issuecomment-889253893
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 5m 41s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | No case conflicting files found. |
| +1 :green_heart: | hbaseanti | 0m 0s | Patch does not have any anti-patterns. |
| +1 :green_heart: | @author | 0m 0s | The patch does not contain any @author tags. |
| -1 :x: | test4tests | 0m 0s | The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. |
||| _ master Compile Tests _ |
| +0 :ok: | mvndep | 5m 5s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 19m 42s | master passed |
| +0 | hbaserecompile | 33m 45s | HBase recompiled. |
| +1 :green_heart: | compile | 1m 51s | master passed |
| +1 :green_heart: | checkstyle | 1m 9s | master passed |
| +1 :green_heart: | javadoc | 1m 26s | master passed |
| +0 :ok: | spotbugs | 4m 18s | phoenix-core in master has 970 extant spotbugs warnings. |
| +0 :ok: | spotbugs | 0m 52s | phoenix-tools in master has 3 extant spotbugs warnings. |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 13s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 16m 34s | the patch passed |
| +0 | hbaserecompile | 23m 34s | HBase recompiled. |
| +1 :green_heart: | compile | 1m 46s | the patch passed |
| +1 :green_heart: | javac | 1m 46s | the patch passed |
| -1 :x: | checkstyle | 0m 46s | phoenix-core: The patch generated 65 new + 909 unchanged - 27 fixed = 974 total (was 936) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | javadoc | 1m 16s | the patch passed |
| +1 :green_heart: | spotbugs | 4m 59s | the patch passed |
||| _ Other Tests _ |
| -1 :x: | unit | 123m 16s | phoenix-core in the patch failed. |
| -1 :x: | unit | 3m 58s | phoenix-tools in the patch failed. |
| +1 :green_heart: | asflicense | 1m 24s | The patch does not generate ASF License warnings. |
| | | 214m 11s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1275/2/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/phoenix/pull/1275 |
| Optional Tests | dupname asflicense javac javadoc unit spotbugs hbaserebuild hbaseanti checkstyle compile |
| uname | Linux da3954fc0345 4.15.0-136-generic #140-Ubuntu SMP Thu Jan 28 05:20:47 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev/phoenix-personality.sh |
| git revision | master / d04b3d8 |
| Default Java | Private Build-1.8.0_242-8u242-b08-0ubuntu3~16.04-b08 |
| checkstyle | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1275/2/artifact/yetus-general-check/output/diff-checkstyle-phoenix-core.txt |
| unit | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1275/2/artifact/yetus-general-check/output/patch-unit-phoenix-core.txt |
| unit | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1275/2/artifact/yetus-general-check/output/patch-unit-phoenix-tools.txt |
| Test Results | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1275/2/testReport/ |
| Max. process+thread count | 13954 (vs. ulimit of 30000) |
| modules | C: phoenix-core phoenix-tools U: . |
| Console output | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1275/2/console |
| versions | git=2.7.4 maven=3.3.9 spotbugs=4.1.3 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@phoenix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] richardantal commented on a change in pull request #1275: PHOENIX-6519 Make SchemaTool work with lower case table and column names
Posted by GitBox <gi...@apache.org>.
richardantal commented on a change in pull request #1275:
URL: https://github.com/apache/phoenix/pull/1275#discussion_r679001770
##########
File path: phoenix-core/src/main/java/org/apache/phoenix/util/SchemaUtil.java
##########
@@ -1238,6 +1238,78 @@ public static String getNormalizedColumnName(ColumnParseNode columnParseNode) {
return columnParseNode.getName();
}
+ public static String getFullTableNameWithQuotes(String schemaName, String tableName,
+ boolean schemaNameCaseSensitive, boolean tableNameCaseSensitive) {
+ String fullTableName;
+
+ if (tableNameCaseSensitive) {
+ fullTableName = "\"" + tableName + "\"";
+ } else {
+ fullTableName = tableName;
+ }
+
+ if(schemaName != null && schemaName.length() != 0) {
+ if (schemaNameCaseSensitive) {
+ fullTableName = "\"" + schemaName + "\"" + QueryConstants.NAME_SEPARATOR + fullTableName;
+ } else {
+ fullTableName = schemaName + QueryConstants.NAME_SEPARATOR + fullTableName;
+ }
+ }
+ return fullTableName;
+ }
+
+ public static String getFullTableNameWithQuotes(String schemaName, String tableName) {
+ return getFullTableNameWithQuotes(schemaName, tableName,
+ quotesNeededForSchema(schemaName), quotesNeededForTable(tableName));
+ }
+
+ private static boolean quotesNeededForSchema(String name) {
+ if (Strings.isNullOrEmpty(name) || name.equals(QueryConstants.DEFAULT_COLUMN_FAMILY)) {
+ return false;
+ }
+ return quotesNeededForTable(name);
+ }
+
+ private static boolean quotesNeededForColumn(String name) {
+ if (!name.equals("_INDEX_ID") && "_".equals(String.valueOf(name.charAt(0)))) {
Review comment:
Sure, I'll upload a new change soon.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@phoenix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] richardantal commented on a change in pull request #1275: PHOENIX-6519 Make SchemaTool work with lower case table and column names
Posted by GitBox <gi...@apache.org>.
richardantal commented on a change in pull request #1275:
URL: https://github.com/apache/phoenix/pull/1275#discussion_r679145324
##########
File path: phoenix-core/src/main/java/org/apache/phoenix/util/SchemaUtil.java
##########
@@ -1238,6 +1238,78 @@ public static String getNormalizedColumnName(ColumnParseNode columnParseNode) {
return columnParseNode.getName();
}
+ public static String getFullTableNameWithQuotes(String schemaName, String tableName,
+ boolean schemaNameCaseSensitive, boolean tableNameCaseSensitive) {
+ String fullTableName;
+
+ if (tableNameCaseSensitive) {
+ fullTableName = "\"" + tableName + "\"";
+ } else {
+ fullTableName = tableName;
+ }
+
+ if(schemaName != null && schemaName.length() != 0) {
+ if (schemaNameCaseSensitive) {
+ fullTableName = "\"" + schemaName + "\"" + QueryConstants.NAME_SEPARATOR + fullTableName;
+ } else {
+ fullTableName = schemaName + QueryConstants.NAME_SEPARATOR + fullTableName;
+ }
+ }
+ return fullTableName;
+ }
+
+ public static String getFullTableNameWithQuotes(String schemaName, String tableName) {
+ return getFullTableNameWithQuotes(schemaName, tableName,
+ quotesNeededForSchema(schemaName), quotesNeededForTable(tableName));
+ }
+
+ private static boolean quotesNeededForSchema(String name) {
+ if (Strings.isNullOrEmpty(name) || name.equals(QueryConstants.DEFAULT_COLUMN_FAMILY)) {
+ return false;
+ }
+ return quotesNeededForTable(name);
+ }
+
+ private static boolean quotesNeededForColumn(String name) {
+ if (!name.equals("_INDEX_ID") && name.startsWith("_")) {
+ return true;
+ }
+ return isQuotesNeeded(name) || containsLowerCase(name);
+ }
+
+ private static boolean quotesNeededForTable(String name) {
+ if (name.startsWith("_")) {
+ return true;
+ }
+ return isQuotesNeeded(name) || containsLowerCase(name);
+ }
+
+ public static String formatSchemaName(String name) {
+ if (quotesNeededForSchema(name)) {
Review comment:
It does, in `quotesNeededForSchema` we check for some extra schema specific stuff but I call the `quotesNeededForTable` there.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@phoenix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org