swaroopak commented on a change in pull request #1233:
URL: https://github.com/apache/phoenix/pull/1233#discussion_r649380413



##########
File path: 
phoenix-tools/src/main/java/org/apache/phoenix/schema/SchemaSynthesisProcessor.java
##########
@@ -154,7 +156,24 @@ private CreateTableStatement 
getCreateTableStatement(AddColumnStatement alterSta
         } else {
             newColDef.addAll(oldColDef);
             newColDef.addAll(addStmt.getColumnDefs());
-            newCreateStmt = new CreateTableStatement(createStmt, newColDef);
+            PrimaryKeyConstraint oldPKConstraint = 
createStmt.getPrimaryKeyConstraint();
+            List<ColumnDefInPkConstraint> pkList = new ArrayList<>();
+            for(Pair<ColumnName, SortOrder> entry : 
oldPKConstraint.getColumnNames()) {
+                ColumnDefInPkConstraint cd = new
+                        ColumnDefInPkConstraint(entry.getFirst(), 
entry.getSecond(), oldPKConstraint.isColumnRowTimestamp(entry
+                .getFirst()));
+                pkList.add(cd);
+            }
+            for(ColumnDef cd : addStmt.getColumnDefs()) {

Review comment:
       That's a valid point, I am thinking to create a new Jira which will 
validate if the statements are valid on the mini-cluster (as parsing won't 
catch this and similar problem) and then only proceed for synthesis. Does that 
sound okay?




-- 
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]


Reply via email to