[ https://issues.apache.org/jira/browse/HIVE-15705?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16103291#comment-16103291 ]
Sankar Hariappan edited comment on HIVE-15705 at 7/27/17 2:46 PM: ------------------------------------------------------------------ [~daijy] Please find my comments below, 1. add_foreign_key, add_primary_key, drop_constraint methods need to rollback the txn for any failures and exceptions. 2. create_table_core - The indexing of constraintNames for notNullConstraints should use (primaryKeySize + foreignKeySize + uniqueConstraintSize + i). May be better to use another iterator variable to traverse constraintNames. 3. @param for the definition of MessageFactory.buildAddNotNullConstraintMessage should be nns. 4. In case of failures, shouldn’t invoke notifyEvent on listeners which is there in finally block. 5. In add_primary_key, primaryKeyCols is accessed in a loop without null check. Similarly, foreignKeyCols in add_foreign_key 6. Events are not logged from add_unique_constraint and add_not_null_constraint. 7. In HbaseStore.constraintNameAlreadyExists, may be need to compare the constraint name using equalsIgnoreCase. 8. In HbaseStore.constraintNameAlreadyExists, the foreign key names can be null if KeySeq != . So, need to validate before accessing it. 9. In HbaseStore and ObjectStore; addForeignKeys, addPrimaryKeys and addUniqueConstraints methods, the constraintName local variable should be set to null at the beginning of each iteration to avoid setting stale value from previous iteration. If user input is null, then set the constraintName only for KeySeq==1. 10. For HbaseStore, why do we need to explicitly generate the constraint names if it is not used while adding it? 11. In MessageDeserializer.getEventMessage, some of the events are missing. 12. In load.message.AddPrimaryKeyHandler, msg.getDb() is used to get the Db name. But, it is not set in the dump side. Instead shall get it from keys itself just like table name. Same with other newly added handlers. 13. For all newly added handlers, need to mark tablesUpdated map as well as table is the immediate parent for constraints. 14. Need to enable idempotent behaviour for constraints by validating allowOperationInReplicationScope before allowing add/drop constraints. Also, need to update the test cases testIncrementalRepeatEventOnExistingObject and testIncrementalRepeatEventOnMissingObject for this. 15. As we store the table name part of Constraint object in metastore, does it need update when rename table is done? cc [~thejas] was (Author: sankarh): [~daijy] Please find my comments below, 1. add_foreign_key, add_primary_key, drop_constraint methods need to rollback the txn for any failures and exceptions. 2. create_table_core - The indexing of constraintNames for notNullConstraints should use (primaryKeySize + foreignKeySize + uniqueConstraintSize + i). May be better to use another iterator variable to traverse constraintNames. 3. @param for the definition of MessageFactory.buildAddNotNullConstraintMessage should be nns. 4. In case of failures, shouldn’t invoke notifyEvent on listeners which is there in finally block. 5. In add_primary_key, primaryKeyCols is accessed in a loop without null check. Similarly, foreignKeyCols in add_foreign_key 6. Events are not logged from add_unique_constraint and add_not_null_constraint. 7. In HbaseStore.constraintNameAlreadyExists, may be need to compare the constraint name using equalsIgnoreCase. 8. In HbaseStore.constraintNameAlreadyExists, the foreign key names can be null if KeySeq != . So, need to validate before accessing it. 9. In HbaseStore and ObjectStore; addForeignKeys, addPrimaryKeys and addUniqueConstraints methods, the constraintName local variable should be set to null at the beginning of each iteration to avoid setting stale value from previous iteration. If user input is null, then set the constraintName only for KeySeq==1. 10. For HbaseStore, why do we need to explicitly generate the constraint names if it is not used while adding it? 11. In MessageDeserializer.getEventMessage, some of the events are missing. 12. In load.message.AddPrimaryKeyHandler, msg.getDb() is used to get the Db name. But, it is not set in the dump side. Instead shall get it from keys itself just like table name. Same with other newly added handlers. 13. For all newly added handlers, need to mark tablesUpdated map as well as table is the immediate parent for constraints. 14. Need to enable idempotent behaviour for constraints by validating allowOperationInReplicationScope before allowing add/drop constraints. 15. As we store the table name part of Constraint object in metastore, does it need update when rename table is done? cc [~thejas] > Event replication for constraints > --------------------------------- > > Key: HIVE-15705 > URL: https://issues.apache.org/jira/browse/HIVE-15705 > Project: Hive > Issue Type: Sub-task > Components: repl > Reporter: Daniel Dai > Assignee: Daniel Dai > Attachments: HIVE-15705.1.patch, HIVE-15705.2.patch, > HIVE-15705.3.patch, HIVE-15705.4.patch > > > Make event replication for primary key and foreign key work. -- This message was sent by Atlassian JIRA (v6.4.14#64029)