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 2020/09/22 00:04:15 UTC
[GitHub] [phoenix] swaroopak opened a new pull request #889: PHOENIX-6148 & PHOENIX-6149 : [SchemaExtractionTool] DDL parsing exception in Phoenix…
swaroopak opened a new pull request #889:
URL: https://github.com/apache/phoenix/pull/889
… in view 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] priyankporwal commented on a change in pull request #889: PHOENIX-6148 & PHOENIX-6149 : [SchemaExtractionTool] DDL parsing exception in Phoenix…
Posted by GitBox <gi...@apache.org>.
priyankporwal commented on a change in pull request #889:
URL: https://github.com/apache/phoenix/pull/889#discussion_r492924434
##########
File path: phoenix-tools/src/main/java/org/apache/phoenix/schema/SchemaExtractionProcessor.java
##########
@@ -202,12 +202,20 @@ protected String extractCreateViewDDL(PTable table) throws SQLException {
private String generateCreateViewDDL(String columnInfoString, String baseTableFullName,
String whereClause, String pSchemaName, String pTableName) {
- String viewFullName = SchemaUtil.getQualifiedTableName(pSchemaName, pTableName);
+ String viewFullName = getPTableFullName(pSchemaName, pTableName);
StringBuilder outputBuilder = new StringBuilder(String.format(CREATE_VIEW, viewFullName,
columnInfoString, baseTableFullName, whereClause));
return outputBuilder.toString();
}
+ private String getPTableFullName(String pSchemaName, String pTableName) {
Review comment:
Why not add this function to SchemaUtil class itself?
##########
File path: phoenix-tools/src/main/java/org/apache/phoenix/schema/SchemaExtractionProcessor.java
##########
@@ -202,12 +202,20 @@ protected String extractCreateViewDDL(PTable table) throws SQLException {
private String generateCreateViewDDL(String columnInfoString, String baseTableFullName,
String whereClause, String pSchemaName, String pTableName) {
- String viewFullName = SchemaUtil.getQualifiedTableName(pSchemaName, pTableName);
+ String viewFullName = getPTableFullName(pSchemaName, pTableName);
StringBuilder outputBuilder = new StringBuilder(String.format(CREATE_VIEW, viewFullName,
columnInfoString, baseTableFullName, whereClause));
return outputBuilder.toString();
}
+ private String getPTableFullName(String pSchemaName, String pTableName) {
Review comment:
Maybe so today, but we never know when it might be needed later. SchemaUtil seems like the right place for it instead of fragmented/duplicated in various tools.
##########
File path: phoenix-tools/src/main/java/org/apache/phoenix/schema/SchemaExtractionProcessor.java
##########
@@ -326,8 +334,13 @@ private String convertPropertiesToString() {
if (optionBuilder.length() != 0) {
optionBuilder.append(", ");
}
- key = columnFamilyName.equals(QueryConstants.DEFAULT_COLUMN_FAMILY)? key : String.format("\"%s\".%s", columnFamilyName, key);
- optionBuilder.append(key+"="+value);
+ key = columnFamilyName.equals(QueryConstants.DEFAULT_COLUMN_FAMILY)?
+ key : String.format("\"%s\".%s", columnFamilyName, key);
+ if(!Character.isDigit(value.charAt(0)) &&
Review comment:
what does digit in the first position signify? Perhaps add a comment as well.
##########
File path: phoenix-tools/src/main/java/org/apache/phoenix/schema/SchemaExtractionProcessor.java
##########
@@ -326,8 +327,15 @@ private String convertPropertiesToString() {
if (optionBuilder.length() != 0) {
optionBuilder.append(", ");
}
- key = columnFamilyName.equals(QueryConstants.DEFAULT_COLUMN_FAMILY)? key : String.format("\"%s\".%s", columnFamilyName, key);
- optionBuilder.append(key+"="+value);
+ key = columnFamilyName.equals(QueryConstants.DEFAULT_COLUMN_FAMILY)?
+ key : String.format("\"%s\".%s", columnFamilyName, key);
+ // properties value that corresponds to a number will not need single quotes around it
Review comment:
Nit: given this comment, shouldn't we check that the string beginning with a digit is actually a number and not something like '72HOURS'? :)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] swaroopak commented on a change in pull request #889: PHOENIX-6148 & PHOENIX-6149 : [SchemaExtractionTool] DDL parsing exception in Phoenix…
Posted by GitBox <gi...@apache.org>.
swaroopak commented on a change in pull request #889:
URL: https://github.com/apache/phoenix/pull/889#discussion_r492429804
##########
File path: phoenix-tools/src/main/java/org/apache/phoenix/schema/SchemaExtractionProcessor.java
##########
@@ -203,6 +203,9 @@ protected String extractCreateViewDDL(PTable table) throws SQLException {
private String generateCreateViewDDL(String columnInfoString, String baseTableFullName,
String whereClause, String pSchemaName, String pTableName) {
String viewFullName = SchemaUtil.getQualifiedTableName(pSchemaName, pTableName);
+ if(Character.isDigit(pTableName.charAt(0))) {
Review comment:
I think not, this is mainly for parsing. For Internals of phoenix, function should ignore quotes around the name.
##########
File path: phoenix-tools/src/main/java/org/apache/phoenix/schema/SchemaExtractionProcessor.java
##########
@@ -203,6 +203,9 @@ protected String extractCreateViewDDL(PTable table) throws SQLException {
private String generateCreateViewDDL(String columnInfoString, String baseTableFullName,
String whereClause, String pSchemaName, String pTableName) {
String viewFullName = SchemaUtil.getQualifiedTableName(pSchemaName, pTableName);
+ if(Character.isDigit(pTableName.charAt(0))) {
Review comment:
Makes sense! Let me correct it.
##########
File path: phoenix-tools/src/main/java/org/apache/phoenix/schema/SchemaExtractionProcessor.java
##########
@@ -203,6 +203,9 @@ protected String extractCreateViewDDL(PTable table) throws SQLException {
private String generateCreateViewDDL(String columnInfoString, String baseTableFullName,
String whereClause, String pSchemaName, String pTableName) {
String viewFullName = SchemaUtil.getQualifiedTableName(pSchemaName, pTableName);
+ if(Character.isDigit(pTableName.charAt(0))) {
Review comment:
Makes sense! However, [a-zA-Z_0-9-.] these are the only acceptable characters in the name.
##########
File path: phoenix-tools/src/main/java/org/apache/phoenix/schema/SchemaExtractionProcessor.java
##########
@@ -202,12 +202,20 @@ protected String extractCreateViewDDL(PTable table) throws SQLException {
private String generateCreateViewDDL(String columnInfoString, String baseTableFullName,
String whereClause, String pSchemaName, String pTableName) {
- String viewFullName = SchemaUtil.getQualifiedTableName(pSchemaName, pTableName);
+ String viewFullName = getPTableFullName(pSchemaName, pTableName);
StringBuilder outputBuilder = new StringBuilder(String.format(CREATE_VIEW, viewFullName,
columnInfoString, baseTableFullName, whereClause));
return outputBuilder.toString();
}
+ private String getPTableFullName(String pSchemaName, String pTableName) {
Review comment:
This is not needed in for internals for phoenix.
##########
File path: phoenix-tools/src/main/java/org/apache/phoenix/schema/SchemaExtractionProcessor.java
##########
@@ -202,12 +202,20 @@ protected String extractCreateViewDDL(PTable table) throws SQLException {
private String generateCreateViewDDL(String columnInfoString, String baseTableFullName,
String whereClause, String pSchemaName, String pTableName) {
- String viewFullName = SchemaUtil.getQualifiedTableName(pSchemaName, pTableName);
+ String viewFullName = getPTableFullName(pSchemaName, pTableName);
StringBuilder outputBuilder = new StringBuilder(String.format(CREATE_VIEW, viewFullName,
columnInfoString, baseTableFullName, whereClause));
return outputBuilder.toString();
}
+ private String getPTableFullName(String pSchemaName, String pTableName) {
Review comment:
This is not needed in for internals of phoenix.
##########
File path: phoenix-tools/src/main/java/org/apache/phoenix/schema/SchemaExtractionProcessor.java
##########
@@ -202,12 +202,20 @@ protected String extractCreateViewDDL(PTable table) throws SQLException {
private String generateCreateViewDDL(String columnInfoString, String baseTableFullName,
String whereClause, String pSchemaName, String pTableName) {
- String viewFullName = SchemaUtil.getQualifiedTableName(pSchemaName, pTableName);
+ String viewFullName = getPTableFullName(pSchemaName, pTableName);
StringBuilder outputBuilder = new StringBuilder(String.format(CREATE_VIEW, viewFullName,
columnInfoString, baseTableFullName, whereClause));
return outputBuilder.toString();
}
+ private String getPTableFullName(String pSchemaName, String pTableName) {
Review comment:
This is not needed in internals of phoenix.
##########
File path: phoenix-tools/src/main/java/org/apache/phoenix/schema/SchemaExtractionProcessor.java
##########
@@ -326,8 +334,13 @@ private String convertPropertiesToString() {
if (optionBuilder.length() != 0) {
optionBuilder.append(", ");
}
- key = columnFamilyName.equals(QueryConstants.DEFAULT_COLUMN_FAMILY)? key : String.format("\"%s\".%s", columnFamilyName, key);
- optionBuilder.append(key+"="+value);
+ key = columnFamilyName.equals(QueryConstants.DEFAULT_COLUMN_FAMILY)?
+ key : String.format("\"%s\".%s", columnFamilyName, key);
+ if(!Character.isDigit(value.charAt(0)) &&
Review comment:
added a comment.
##########
File path: phoenix-tools/src/main/java/org/apache/phoenix/schema/SchemaExtractionProcessor.java
##########
@@ -326,8 +327,15 @@ private String convertPropertiesToString() {
if (optionBuilder.length() != 0) {
optionBuilder.append(", ");
}
- key = columnFamilyName.equals(QueryConstants.DEFAULT_COLUMN_FAMILY)? key : String.format("\"%s\".%s", columnFamilyName, key);
- optionBuilder.append(key+"="+value);
+ key = columnFamilyName.equals(QueryConstants.DEFAULT_COLUMN_FAMILY)?
+ key : String.format("\"%s\".%s", columnFamilyName, key);
+ // properties value that corresponds to a number will not need single quotes around it
Review comment:
That makes sense, I changed it.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] swaroopak commented on a change in pull request #889: PHOENIX-6148 & PHOENIX-6149 : [SchemaExtractionTool] DDL parsing exception in Phoenix…
Posted by GitBox <gi...@apache.org>.
swaroopak commented on a change in pull request #889:
URL: https://github.com/apache/phoenix/pull/889#discussion_r492429804
##########
File path: phoenix-tools/src/main/java/org/apache/phoenix/schema/SchemaExtractionProcessor.java
##########
@@ -203,6 +203,9 @@ protected String extractCreateViewDDL(PTable table) throws SQLException {
private String generateCreateViewDDL(String columnInfoString, String baseTableFullName,
String whereClause, String pSchemaName, String pTableName) {
String viewFullName = SchemaUtil.getQualifiedTableName(pSchemaName, pTableName);
+ if(Character.isDigit(pTableName.charAt(0))) {
Review comment:
I think not, this is mainly for parsing. For Internals of phoenix, function should ignore quotes around the 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] priyankporwal commented on a change in pull request #889: PHOENIX-6148 & PHOENIX-6149 : [SchemaExtractionTool] DDL parsing exception in Phoenix…
Posted by GitBox <gi...@apache.org>.
priyankporwal commented on a change in pull request #889:
URL: https://github.com/apache/phoenix/pull/889#discussion_r492924434
##########
File path: phoenix-tools/src/main/java/org/apache/phoenix/schema/SchemaExtractionProcessor.java
##########
@@ -202,12 +202,20 @@ protected String extractCreateViewDDL(PTable table) throws SQLException {
private String generateCreateViewDDL(String columnInfoString, String baseTableFullName,
String whereClause, String pSchemaName, String pTableName) {
- String viewFullName = SchemaUtil.getQualifiedTableName(pSchemaName, pTableName);
+ String viewFullName = getPTableFullName(pSchemaName, pTableName);
StringBuilder outputBuilder = new StringBuilder(String.format(CREATE_VIEW, viewFullName,
columnInfoString, baseTableFullName, whereClause));
return outputBuilder.toString();
}
+ private String getPTableFullName(String pSchemaName, String pTableName) {
Review comment:
Why not add this function to SchemaUtil class itself?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] swaroopak commented on a change in pull request #889: PHOENIX-6148 & PHOENIX-6149 : [SchemaExtractionTool] DDL parsing exception in Phoenix…
Posted by GitBox <gi...@apache.org>.
swaroopak commented on a change in pull request #889:
URL: https://github.com/apache/phoenix/pull/889#discussion_r493024134
##########
File path: phoenix-tools/src/main/java/org/apache/phoenix/schema/SchemaExtractionProcessor.java
##########
@@ -326,8 +334,13 @@ private String convertPropertiesToString() {
if (optionBuilder.length() != 0) {
optionBuilder.append(", ");
}
- key = columnFamilyName.equals(QueryConstants.DEFAULT_COLUMN_FAMILY)? key : String.format("\"%s\".%s", columnFamilyName, key);
- optionBuilder.append(key+"="+value);
+ key = columnFamilyName.equals(QueryConstants.DEFAULT_COLUMN_FAMILY)?
+ key : String.format("\"%s\".%s", columnFamilyName, key);
+ if(!Character.isDigit(value.charAt(0)) &&
Review comment:
added a comment.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] swaroopak commented on a change in pull request #889: PHOENIX-6148 & PHOENIX-6149 : [SchemaExtractionTool] DDL parsing exception in Phoenix…
Posted by GitBox <gi...@apache.org>.
swaroopak commented on a change in pull request #889:
URL: https://github.com/apache/phoenix/pull/889#discussion_r492925029
##########
File path: phoenix-tools/src/main/java/org/apache/phoenix/schema/SchemaExtractionProcessor.java
##########
@@ -202,12 +202,20 @@ protected String extractCreateViewDDL(PTable table) throws SQLException {
private String generateCreateViewDDL(String columnInfoString, String baseTableFullName,
String whereClause, String pSchemaName, String pTableName) {
- String viewFullName = SchemaUtil.getQualifiedTableName(pSchemaName, pTableName);
+ String viewFullName = getPTableFullName(pSchemaName, pTableName);
StringBuilder outputBuilder = new StringBuilder(String.format(CREATE_VIEW, viewFullName,
columnInfoString, baseTableFullName, whereClause));
return outputBuilder.toString();
}
+ private String getPTableFullName(String pSchemaName, String pTableName) {
Review comment:
This is not needed in for internals for phoenix.
##########
File path: phoenix-tools/src/main/java/org/apache/phoenix/schema/SchemaExtractionProcessor.java
##########
@@ -202,12 +202,20 @@ protected String extractCreateViewDDL(PTable table) throws SQLException {
private String generateCreateViewDDL(String columnInfoString, String baseTableFullName,
String whereClause, String pSchemaName, String pTableName) {
- String viewFullName = SchemaUtil.getQualifiedTableName(pSchemaName, pTableName);
+ String viewFullName = getPTableFullName(pSchemaName, pTableName);
StringBuilder outputBuilder = new StringBuilder(String.format(CREATE_VIEW, viewFullName,
columnInfoString, baseTableFullName, whereClause));
return outputBuilder.toString();
}
+ private String getPTableFullName(String pSchemaName, String pTableName) {
Review comment:
This is not needed in for internals of phoenix.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] priyankporwal commented on a change in pull request #889: PHOENIX-6148 & PHOENIX-6149 : [SchemaExtractionTool] DDL parsing exception in Phoenix…
Posted by GitBox <gi...@apache.org>.
priyankporwal commented on a change in pull request #889:
URL: https://github.com/apache/phoenix/pull/889#discussion_r493012125
##########
File path: phoenix-tools/src/main/java/org/apache/phoenix/schema/SchemaExtractionProcessor.java
##########
@@ -202,12 +202,20 @@ protected String extractCreateViewDDL(PTable table) throws SQLException {
private String generateCreateViewDDL(String columnInfoString, String baseTableFullName,
String whereClause, String pSchemaName, String pTableName) {
- String viewFullName = SchemaUtil.getQualifiedTableName(pSchemaName, pTableName);
+ String viewFullName = getPTableFullName(pSchemaName, pTableName);
StringBuilder outputBuilder = new StringBuilder(String.format(CREATE_VIEW, viewFullName,
columnInfoString, baseTableFullName, whereClause));
return outputBuilder.toString();
}
+ private String getPTableFullName(String pSchemaName, String pTableName) {
Review comment:
Maybe so today, but we never know when it might be needed later. SchemaUtil seems like the right place for it instead of fragmented/duplicated in various tools.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] swaroopak commented on a change in pull request #889: PHOENIX-6148 & PHOENIX-6149 : [SchemaExtractionTool] DDL parsing exception in Phoenix…
Posted by GitBox <gi...@apache.org>.
swaroopak commented on a change in pull request #889:
URL: https://github.com/apache/phoenix/pull/889#discussion_r492429804
##########
File path: phoenix-tools/src/main/java/org/apache/phoenix/schema/SchemaExtractionProcessor.java
##########
@@ -203,6 +203,9 @@ protected String extractCreateViewDDL(PTable table) throws SQLException {
private String generateCreateViewDDL(String columnInfoString, String baseTableFullName,
String whereClause, String pSchemaName, String pTableName) {
String viewFullName = SchemaUtil.getQualifiedTableName(pSchemaName, pTableName);
+ if(Character.isDigit(pTableName.charAt(0))) {
Review comment:
I think not, this is mainly for parsing. For Internals of phoenix, function should ignore quotes around the 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] gjacoby126 commented on a change in pull request #889: PHOENIX-6148 & PHOENIX-6149 : [SchemaExtractionTool] DDL parsing exception in Phoenix…
Posted by GitBox <gi...@apache.org>.
gjacoby126 commented on a change in pull request #889:
URL: https://github.com/apache/phoenix/pull/889#discussion_r492429045
##########
File path: phoenix-tools/src/main/java/org/apache/phoenix/schema/SchemaExtractionProcessor.java
##########
@@ -203,6 +203,9 @@ protected String extractCreateViewDDL(PTable table) throws SQLException {
private String generateCreateViewDDL(String columnInfoString, String baseTableFullName,
String whereClause, String pSchemaName, String pTableName) {
String viewFullName = SchemaUtil.getQualifiedTableName(pSchemaName, pTableName);
+ if(Character.isDigit(pTableName.charAt(0))) {
Review comment:
Is this a bug in SchemaUtil.getQualfiedTableName? This seems like something we shouldn't have to repeat everywhere.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] priyankporwal commented on a change in pull request #889: PHOENIX-6148 & PHOENIX-6149 : [SchemaExtractionTool] DDL parsing exception in Phoenix…
Posted by GitBox <gi...@apache.org>.
priyankporwal commented on a change in pull request #889:
URL: https://github.com/apache/phoenix/pull/889#discussion_r493029846
##########
File path: phoenix-tools/src/main/java/org/apache/phoenix/schema/SchemaExtractionProcessor.java
##########
@@ -326,8 +327,15 @@ private String convertPropertiesToString() {
if (optionBuilder.length() != 0) {
optionBuilder.append(", ");
}
- key = columnFamilyName.equals(QueryConstants.DEFAULT_COLUMN_FAMILY)? key : String.format("\"%s\".%s", columnFamilyName, key);
- optionBuilder.append(key+"="+value);
+ key = columnFamilyName.equals(QueryConstants.DEFAULT_COLUMN_FAMILY)?
+ key : String.format("\"%s\".%s", columnFamilyName, key);
+ // properties value that corresponds to a number will not need single quotes around it
Review comment:
Nit: given this comment, shouldn't we check that the string beginning with a digit is actually a number and not something like '72HOURS'? :)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] swaroopak commented on a change in pull request #889: PHOENIX-6148 & PHOENIX-6149 : [SchemaExtractionTool] DDL parsing exception in Phoenix…
Posted by GitBox <gi...@apache.org>.
swaroopak commented on a change in pull request #889:
URL: https://github.com/apache/phoenix/pull/889#discussion_r492925320
##########
File path: phoenix-tools/src/main/java/org/apache/phoenix/schema/SchemaExtractionProcessor.java
##########
@@ -202,12 +202,20 @@ protected String extractCreateViewDDL(PTable table) throws SQLException {
private String generateCreateViewDDL(String columnInfoString, String baseTableFullName,
String whereClause, String pSchemaName, String pTableName) {
- String viewFullName = SchemaUtil.getQualifiedTableName(pSchemaName, pTableName);
+ String viewFullName = getPTableFullName(pSchemaName, pTableName);
StringBuilder outputBuilder = new StringBuilder(String.format(CREATE_VIEW, viewFullName,
columnInfoString, baseTableFullName, whereClause));
return outputBuilder.toString();
}
+ private String getPTableFullName(String pSchemaName, String pTableName) {
Review comment:
This is not needed in internals of phoenix.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] swaroopak commented on a change in pull request #889: PHOENIX-6148 & PHOENIX-6149 : [SchemaExtractionTool] DDL parsing exception in Phoenix…
Posted by GitBox <gi...@apache.org>.
swaroopak commented on a change in pull request #889:
URL: https://github.com/apache/phoenix/pull/889#discussion_r492904710
##########
File path: phoenix-tools/src/main/java/org/apache/phoenix/schema/SchemaExtractionProcessor.java
##########
@@ -203,6 +203,9 @@ protected String extractCreateViewDDL(PTable table) throws SQLException {
private String generateCreateViewDDL(String columnInfoString, String baseTableFullName,
String whereClause, String pSchemaName, String pTableName) {
String viewFullName = SchemaUtil.getQualifiedTableName(pSchemaName, pTableName);
+ if(Character.isDigit(pTableName.charAt(0))) {
Review comment:
Makes sense! However, [a-zA-Z_0-9-.] these are the only acceptable characters in the 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] priyankporwal commented on a change in pull request #889: PHOENIX-6148 & PHOENIX-6149 : [SchemaExtractionTool] DDL parsing exception in Phoenix…
Posted by GitBox <gi...@apache.org>.
priyankporwal commented on a change in pull request #889:
URL: https://github.com/apache/phoenix/pull/889#discussion_r493014166
##########
File path: phoenix-tools/src/main/java/org/apache/phoenix/schema/SchemaExtractionProcessor.java
##########
@@ -326,8 +334,13 @@ private String convertPropertiesToString() {
if (optionBuilder.length() != 0) {
optionBuilder.append(", ");
}
- key = columnFamilyName.equals(QueryConstants.DEFAULT_COLUMN_FAMILY)? key : String.format("\"%s\".%s", columnFamilyName, key);
- optionBuilder.append(key+"="+value);
+ key = columnFamilyName.equals(QueryConstants.DEFAULT_COLUMN_FAMILY)?
+ key : String.format("\"%s\".%s", columnFamilyName, key);
+ if(!Character.isDigit(value.charAt(0)) &&
Review comment:
what does digit in the first position signify? Perhaps add a comment 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] tkhurana commented on a change in pull request #889: PHOENIX-6148 & PHOENIX-6149 : [SchemaExtractionTool] DDL parsing exception in Phoenix…
Posted by GitBox <gi...@apache.org>.
tkhurana commented on a change in pull request #889:
URL: https://github.com/apache/phoenix/pull/889#discussion_r492893713
##########
File path: phoenix-tools/src/main/java/org/apache/phoenix/schema/SchemaExtractionProcessor.java
##########
@@ -203,6 +203,9 @@ protected String extractCreateViewDDL(PTable table) throws SQLException {
private String generateCreateViewDDL(String columnInfoString, String baseTableFullName,
String whereClause, String pSchemaName, String pTableName) {
String viewFullName = SchemaUtil.getQualifiedTableName(pSchemaName, pTableName);
+ if(Character.isDigit(pTableName.charAt(0))) {
Review comment:
If you look at the phoenix grammar for create table or create view the table name is of the format below: https://phoenix.apache.org/language/index.html#table_ref
https://phoenix.apache.org/language/index.html#name
So table names can begin with [A-Z] or _ so any table name that doesn't begin with this has to begin with a quote. So just checking for `isDigit` is wrong as we might miss other cases where the first letter is something other than a digit for example like a `$` or other character. The fix should be if the first character is not a letter or underscore then quote.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] gjacoby126 commented on a change in pull request #889: PHOENIX-6148 & PHOENIX-6149 : [SchemaExtractionTool] DDL parsing exception in Phoenix…
Posted by GitBox <gi...@apache.org>.
gjacoby126 commented on a change in pull request #889:
URL: https://github.com/apache/phoenix/pull/889#discussion_r492429045
##########
File path: phoenix-tools/src/main/java/org/apache/phoenix/schema/SchemaExtractionProcessor.java
##########
@@ -203,6 +203,9 @@ protected String extractCreateViewDDL(PTable table) throws SQLException {
private String generateCreateViewDDL(String columnInfoString, String baseTableFullName,
String whereClause, String pSchemaName, String pTableName) {
String viewFullName = SchemaUtil.getQualifiedTableName(pSchemaName, pTableName);
+ if(Character.isDigit(pTableName.charAt(0))) {
Review comment:
Is this a bug in SchemaUtil.getQualfiedTableName? This seems like something we shouldn't have to repeat everywhere.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] gjacoby126 commented on a change in pull request #889: PHOENIX-6148 & PHOENIX-6149 : [SchemaExtractionTool] DDL parsing exception in Phoenix…
Posted by GitBox <gi...@apache.org>.
gjacoby126 commented on a change in pull request #889:
URL: https://github.com/apache/phoenix/pull/889#discussion_r492429045
##########
File path: phoenix-tools/src/main/java/org/apache/phoenix/schema/SchemaExtractionProcessor.java
##########
@@ -203,6 +203,9 @@ protected String extractCreateViewDDL(PTable table) throws SQLException {
private String generateCreateViewDDL(String columnInfoString, String baseTableFullName,
String whereClause, String pSchemaName, String pTableName) {
String viewFullName = SchemaUtil.getQualifiedTableName(pSchemaName, pTableName);
+ if(Character.isDigit(pTableName.charAt(0))) {
Review comment:
Is this a bug in SchemaUtil.getQualfiedTableName? This seems like something we shouldn't have to repeat everywhere.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] tkhurana commented on a change in pull request #889: PHOENIX-6148 & PHOENIX-6149 : [SchemaExtractionTool] DDL parsing exception in Phoenix…
Posted by GitBox <gi...@apache.org>.
tkhurana commented on a change in pull request #889:
URL: https://github.com/apache/phoenix/pull/889#discussion_r492893713
##########
File path: phoenix-tools/src/main/java/org/apache/phoenix/schema/SchemaExtractionProcessor.java
##########
@@ -203,6 +203,9 @@ protected String extractCreateViewDDL(PTable table) throws SQLException {
private String generateCreateViewDDL(String columnInfoString, String baseTableFullName,
String whereClause, String pSchemaName, String pTableName) {
String viewFullName = SchemaUtil.getQualifiedTableName(pSchemaName, pTableName);
+ if(Character.isDigit(pTableName.charAt(0))) {
Review comment:
If you look at the phoenix grammar for create table or create view the table name is of the format below: https://phoenix.apache.org/language/index.html#table_ref
https://phoenix.apache.org/language/index.html#name
So table names can begin with [A-Z] or _ so any table name that doesn't begin with this has to begin with a quote. So just checking for `isDigit` is wrong as we might miss other cases where the first letter is something other than a digit for example like a `$` or other character. The fix should be if the first character is not a letter or underscore then quote.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] swaroopak merged pull request #889: PHOENIX-6148 & PHOENIX-6149 : [SchemaExtractionTool] DDL parsing exception in Phoenix…
Posted by GitBox <gi...@apache.org>.
swaroopak merged pull request #889:
URL: https://github.com/apache/phoenix/pull/889
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] swaroopak commented on a change in pull request #889: PHOENIX-6148 & PHOENIX-6149 : [SchemaExtractionTool] DDL parsing exception in Phoenix…
Posted by GitBox <gi...@apache.org>.
swaroopak commented on a change in pull request #889:
URL: https://github.com/apache/phoenix/pull/889#discussion_r493058032
##########
File path: phoenix-tools/src/main/java/org/apache/phoenix/schema/SchemaExtractionProcessor.java
##########
@@ -326,8 +327,15 @@ private String convertPropertiesToString() {
if (optionBuilder.length() != 0) {
optionBuilder.append(", ");
}
- key = columnFamilyName.equals(QueryConstants.DEFAULT_COLUMN_FAMILY)? key : String.format("\"%s\".%s", columnFamilyName, key);
- optionBuilder.append(key+"="+value);
+ key = columnFamilyName.equals(QueryConstants.DEFAULT_COLUMN_FAMILY)?
+ key : String.format("\"%s\".%s", columnFamilyName, key);
+ // properties value that corresponds to a number will not need single quotes around it
Review comment:
That makes sense, I changed it.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] swaroopak commented on a change in pull request #889: PHOENIX-6148 & PHOENIX-6149 : [SchemaExtractionTool] DDL parsing exception in Phoenix…
Posted by GitBox <gi...@apache.org>.
swaroopak commented on a change in pull request #889:
URL: https://github.com/apache/phoenix/pull/889#discussion_r492904710
##########
File path: phoenix-tools/src/main/java/org/apache/phoenix/schema/SchemaExtractionProcessor.java
##########
@@ -203,6 +203,9 @@ protected String extractCreateViewDDL(PTable table) throws SQLException {
private String generateCreateViewDDL(String columnInfoString, String baseTableFullName,
String whereClause, String pSchemaName, String pTableName) {
String viewFullName = SchemaUtil.getQualifiedTableName(pSchemaName, pTableName);
+ if(Character.isDigit(pTableName.charAt(0))) {
Review comment:
Makes sense! Let me correct it.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org