Thanks, Jinfeng. I added the extra test; everything passes now and https://issues.apache.org/jira/browse/CALCITE-1208 <https://issues.apache.org/jira/browse/CALCITE-1208> is ready for review.
> On Jul 26, 2016, at 3:59 PM, Jinfeng Ni <[email protected]> wrote: > > @Julian, > > The new test added to test checkAmbiguousUnresolvedStar() looks good > to me. I forgot to add such testing, when working on Calcite-1150, > though Drill has unit test for such case. > > One more test we may consider adding. When resolve column reference, > regular field has higher priority than dynamic star columns. (I'm not > completely sure if we should allow such case, or block it, since this > column reference may be resolved to the regular field, or a field > expanded later on from * ) > > @Test public void testDynamicStar2() throws Exception { > final String sql = "select newid\n" > + "from (select *, NATION.N_NATION + 100 as newid from > \"DYNAMIC\".NATION, \"DYNAMIC\".CUSTOMER)"; > sql(sql).type("RecordType(ANY NEWID) NOT NULL"); > } > > Thanks again for adding those test. > > > > On Tue, Jul 26, 2016 at 1:40 AM, Julian Hyde <[email protected]> wrote: >> Jinfeng, >> >> Here’s a patch that attempts to test checkAmbiguousUnresolvedStar. Let me >> know whether I’ve missed any coverage. >> >> Julian >> >> >> diff --git >> a/core/src/test/java/org/apache/calcite/test/MockCatalogReader.java >> b/core/src/test/java/org/apache/calcite/test/MockCatalogReader.java >> index e56dd96..2c791cf 100644 >> --- a/core/src/test/java/org/apache/calcite/test/MockCatalogReader.java >> +++ b/core/src/test/java/org/apache/calcite/test/MockCatalogReader.java >> @@ -233,6 +233,20 @@ public MockCatalogReader init() { >> contactAddressTable.addColumn("MAILING_ADDRESS", addressType); >> registerTable(contactAddressTable); >> >> + // Register "DYNAMIC" schema. >> + MockSchema dynamicSchema = new MockSchema("DYNAMIC"); >> + registerSchema(dynamicSchema); >> + >> + MockTable nationTable = >> + new MockDynamicTable(this, dynamicSchema.getCatalogName(), >> + dynamicSchema.getName(), "NATION", false, 100); >> + registerTable(nationTable); >> + >> + MockTable customerTable = >> + new MockDynamicTable(this, dynamicSchema.getCatalogName(), >> + dynamicSchema.getName(), "CUSTOMER", false, 100); >> + registerTable(customerTable); >> + >> // Register "CUSTOMER" schema. >> MockSchema customerSchema = new MockSchema("CUSTOMER"); >> registerSchema(customerSchema); >> diff --git >> a/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java >> b/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java >> index 57f71d1..c095453 100644 >> --- a/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java >> +++ b/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java >> @@ -7777,6 +7777,37 @@ public void _testValuesWithAggFuncs() { >> // 12345678901234567890123456789012345678901234567890123456789012345 >> // check("SELECT count(0) FROM emp GROUP BY ()"); >> } >> + >> + >> + /** Test case for >> + * <a >> href="https://issues.apache.org/jira/browse/CALCITE-1150";>[CALCITE-1150] >> + * Dynamic Table / Dynamic Star support</a>. */ >> + @Test public void testDynamicStar() throws Exception { >> + final String sql = "select n.n_nation\n" >> + + "from (select * from \"DYNAMIC\".NATION) as n,\n" >> + + " (select * from \"DYNAMIC\".CUSTOMER)"; >> + sql(sql).type("RecordType(ANY N_NATION) NOT NULL"); >> + } >> + >> + @Test public void testAmbiguousDynamicStar() throws Exception { >> + final String sql = "select ^n_nation^\n" >> + + "from (select * from \"DYNAMIC\".NATION),\n" >> + + " (select * from \"DYNAMIC\".CUSTOMER)"; >> + sql(sql).fails("Column 'N_NATION' is ambiguous"); >> + } >> + >> + @Test public void testAmbiguousDynamicStar2() throws Exception { >> + final String sql = "select ^n_nation^\n" >> + + "from (select * from \"DYNAMIC\".NATION, \"DYNAMIC\".CUSTOMER)"; >> + sql(sql).fails("Column 'N_NATION' is ambiguous"); >> + } >> + >> + @Test public void testAmbiguousDynamicStar3() throws Exception { >> + final String sql = "select ^nc.n_nation^\n" >> + + "from (select * from \"DYNAMIC\".NATION, \"DYNAMIC\".CUSTOMER) as >> nc"; >> + sql(sql).fails("Column 'N_NATION' is ambiguous"); >> + } >> + >> } >> >> // End SqlValidatorTest.java >> >> >> >>> On Jul 26, 2016, at 1:05 AM, Julian Hyde <[email protected]> wrote: >>> >>> Jinfeng, >>> >>> In https://issues.apache.org/jira/browse/CALCITE-1150 you added dynamic >>> record types, and the method checkAmbiguousUnresolvedStar is supposed to >>> throw “Column ‘X’ is ambiguous” if more than one field has a dynamic star. >>> >>> As part of https://issues.apache.org/jira/browse/CALCITE-1208 I am >>> revisiting that code and doing some major refactoring. >>> >>> Did you add any tests where checkAmbiguousUnresolvedStar throws that >>> particular error? I can’t find any; in fact, if I remove all calls to that >>> method, the test suite still passes. >>> >>> Can you suggest some tests for that functionality? I want to make sure I’m >>> not breaking what you did. >>> >>> Julian >>> >>> >>> >>
