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:
[email protected]
