Uploaded image for project: 'Calcite'
  1. Calcite
  2. CALCITE-1426

Support customized star expansion in Table

    Details

    • Type: Improvement
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 1.11.0
    • Component/s: core
    • Labels:

      Description

      This is to support PHOENIX-3357. Phoenix allows users to define columns in arbitrary order regardless of their column families, for example,

      CREATE TABLE t
        (a_string varchar not null, cf1.a integer, cf1.b varchar, col1 integer, cf2.c varchar, cf2.d integer, col2 integer
        CONSTRAINT pk PRIMARY KEY (a_string))
      

      , in which columns from the same family (i.e., col1 and col2 from the default column family) are not necessarily adjacent to each other.
      As a result, when we return row type for a PhoenixTable, we re-order the columns in order to fit them into the two-level column structure. This works fine in most cases except when:
      1) "upsert into t ..." would require a different row type even after flattening (we do not have flattening so far, still need to implement CALCITE-1425).
      2) select * from t would return a different column order.

        Issue Links

          Activity

          Hide
          julianhyde Julian Hyde added a comment -

          See my comments on PHOENIX-3557. Let's reach consensus there before we start proposing Calcite API changes here.

          Show
          julianhyde Julian Hyde added a comment - See my comments on PHOENIX-3557 . Let's reach consensus there before we start proposing Calcite API changes here.
          Hide
          julianhyde Julian Hyde added a comment -

          It's too intrusive to add a new method to Table. Can you create a sub-interface instead?

          Having the method return lists of strings seems too closely fitted to Phoenix's requirements. (Phoenix is the only database I've ever heard of that has column families. If this is not useful beyond Phoenix maybe we should just add an expandStar method to the validator and let Phoenix override.) Could the method instead return a list of expressions and their aliases? That way we could model system columns, default columns, and other things. Or maybe the method could return a SqlNode that represents a query, e.g.

          select k0, c1, f1.a0 as f1_a0, f2.a0 as f2_a0, f0.c0 f0_c0,
            f1.c0 as f1_c0, f1.c2 as f1_c2, f2.c3 as f2.c3
          from t
          

          and replace "from t" by "from (select k0 ... from t)" if a query requests star expansion. Obviously this is much more general but it means the feature is potentially useful for other than Phoenix. Note that it does save a little code in the validator, because the Table implementation will deal with creating SqlIdentifiers and deducing aliases.

          Show
          julianhyde Julian Hyde added a comment - It's too intrusive to add a new method to Table. Can you create a sub-interface instead? Having the method return lists of strings seems too closely fitted to Phoenix's requirements. (Phoenix is the only database I've ever heard of that has column families. If this is not useful beyond Phoenix maybe we should just add an expandStar method to the validator and let Phoenix override.) Could the method instead return a list of expressions and their aliases? That way we could model system columns, default columns, and other things. Or maybe the method could return a SqlNode that represents a query, e.g. select k0, c1, f1.a0 as f1_a0, f2.a0 as f2_a0, f0.c0 f0_c0, f1.c0 as f1_c0, f1.c2 as f1_c2, f2.c3 as f2.c3 from t and replace "from t" by "from (select k0 ... from t)" if a query requests star expansion. Obviously this is much more general but it means the feature is potentially useful for other than Phoenix. Note that it does save a little code in the validator, because the Table implementation will deal with creating SqlIdentifiers and deducing aliases.
          Hide
          maryannxue Maryann Xue added a comment -

          Yes, I agree that returning a list of expressions would be more general. But we need this interface in two places actually (just pushed another check-in to that branch before I saw this comment):
          1. select * expansion
          2. insert statement where target column list is not present
          So if this interface is going to be used for 2) as well, expressions might not be desired. The reason why I used a list of strings was that I thought introducing SqlNode into the Table interface would seem a bit weird.
          I'm fine with creating a sub-interface, though. Adding a method in SqlValidator also sounds fine, but it's only that Calcite has a few places that creates a SqlValidator instance.

          Show
          maryannxue Maryann Xue added a comment - Yes, I agree that returning a list of expressions would be more general. But we need this interface in two places actually (just pushed another check-in to that branch before I saw this comment): 1. select * expansion 2. insert statement where target column list is not present So if this interface is going to be used for 2) as well, expressions might not be desired. The reason why I used a list of strings was that I thought introducing SqlNode into the Table interface would seem a bit weird. I'm fine with creating a sub-interface, though. Adding a method in SqlValidator also sounds fine, but it's only that Calcite has a few places that creates a SqlValidator instance.
          Hide
          maryannxue Maryann Xue added a comment -

          I have also started to look at CALCITE-1425, which is related to the "insert" part of this issue. It would seem a little bit unnatural to have the INSERT support multiple-level column names, and also TableModify.getUpdateColumnList() would have to return something different from a list of strings.

          Show
          maryannxue Maryann Xue added a comment - I have also started to look at CALCITE-1425 , which is related to the "insert" part of this issue. It would seem a little bit unnatural to have the INSERT support multiple-level column names, and also TableModify.getUpdateColumnList() would have to return something different from a list of strings.
          Hide
          julianhyde Julian Hyde added a comment -

          I agree, returning lists of strings is best for now. But please mark the interface experimental, so we can change our minds.

          Show
          julianhyde Julian Hyde added a comment - I agree, returning lists of strings is best for now. But please mark the interface experimental, so we can change our minds.
          Hide
          maryannxue Maryann Xue added a comment -

          Yes, sure. I created a sub-interface CustomExpansionTable and marked it experimental. Could you please take a look at https://github.com/apache/calcite/pull/307 again?

          Show
          maryannxue Maryann Xue added a comment - Yes, sure. I created a sub-interface CustomExpansionTable and marked it experimental. Could you please take a look at https://github.com/apache/calcite/pull/307 again?
          Hide
          julianhyde Julian Hyde added a comment -

          Two things:

          With those, +1.

          Show
          julianhyde Julian Hyde added a comment - Two things: in getCustomStarExpansion's javadoc, describe why each element is a list of strings, not a string at https://github.com/apache/calcite/pull/307/files#diff-45ba82aede133852105182a67de1c736R466 , move the assignment out of `if` and into the variable declaration With those, +1.
          Hide
          maryannxue Maryann Xue added a comment -
          Show
          maryannxue Maryann Xue added a comment - Thank you very much for the suggestions, Julian Hyde ! Fixed in https://git1-us-west.apache.org/repos/asf?p=calcite.git;a=commit;h=5be0f0b .
          Hide
          julianhyde Julian Hyde added a comment -

          Resolved in release 1.11.0 (2017-01-11).

          Show
          julianhyde Julian Hyde added a comment - Resolved in release 1.11.0 (2017-01-11).

            People

            • Assignee:
              maryannxue Maryann Xue
              Reporter:
              maryannxue Maryann Xue
            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development