|
See comments below. Thanks for the review! Satheesh Daniel John Debrunner wrote: This is not possible. When the second user attemps C -> A, the current code in createAliasConstantAction tries to resolve A to B and then gets blocked trying to get AliasDescriptor for B. There is already synonym cycle detection code, which blocks trying to get AliasDescriptor for B and eventually times-out.- QueryTreeNode.resolveTableToSynonym The for(;;) could use some comments as to why it won't loop forever (circle of synonyms) Are there any issues with circular dependencies being caused by rollbacks? E.g. A->B B->A - dropped but not committed.A->B B->C - dropped but not committed. C->A Then the other user rolls back the drop of B While there is no harm in allowing DROP SYNONYM in soft upgrade mode, I think it is better to raise the same error as CREATE SYNOYM. It would seem odd that DROP SYNONYM would be "accepted" after soft upgrade, but not CREATE SYNONYM. It is better to raise the same error message to say database needs hard upgrade, I think.- sqlgrammar.jj - DROP SYNONYM No need for the checkVersion on the DROP, it doesn't create any on-disk artifacts that would cause problems in earlier releases
These are changed now. I don't dislike repetitive, but don't mind
changing to circular.Next two should be solved by adding a row to SYSTABLES too. I will use same order of access to catalogs, SYSTABLES followed by SYSALIASES (for synonym) for both create table and create synonym statements. Hopefully this should avoid any locking issues when the rows are not cached. I have also changed dblook to recognise synonyms. I will attach the patch to Jira and commit it on Monday at around noon time (Pacific Time) if there are no other comments. I can address any comments received later as a separate patch, if needed. Satheesh - OTHER --- I think more checking is need for the CREATE TABLE and CREATE VIEW to ensure the name does not exist as a SYNONYM. Unless I missed it, you are only performing the check at compile time, not runtime. A create statement can be prepared and then execute later, e.g. prepare CT as 'CREATE TABLE T1(I INT)'; CREATE SYNONYM T1; execute CT; These checks would be in the constant actions for these operations. --- As you mentioned the potential for deadlocks or duplicate names on the namespace across two tables (SYSTABLES and SYSALIAS) might exist. A lot depends on when you request a table or alias descriptor from the data dictionary is an actual lock held on the row, or is it a read committed lock, or is it simply a cache lookup? Currently this is enforced at the end of the day for tables and views by the unique index on SYSTABLES. My guess is a stress test may hit problems with your approach. My only thought on this would be to have a row in SYSTABLES for the SYNONYM to ensure uniqeness, and a row in SYSALIASes. But that's just off the top of my head, there may be similar problems with such an approach. --- Is there a jira entry for this, could future patches be attached to that entry? Dan. |
