substrait-io / substrait-java

Apache License 2.0
72 stars 70 forks source link

feat(isthmus): support for SQL expressions in CLI #209

Closed davisusanibar closed 5 months ago

davisusanibar commented 7 months ago

To close https://github.com/substrait-io/substrait-java/issues/207

davisusanibar commented 7 months ago

This PR depend of: https://github.com/substrait-io/substrait-java/pull/206 / https://github.com/substrait-io/substrait-java/pull/191

davisusanibar commented 6 months ago

Consider the following:

As part of this PR, SQL Expression will also be converted into Subtrait Extended Expression. In order to achieve this, a more generic class is needed that offers parameters to identify whether a conversion is to a Substrat Plan or Extended Expression (it is backwards compatible with current functionality).

davisusanibar commented 6 months ago

Examples of SQL Expression into Substrait Extended Expression:

New Isthmus info:

❯ ./isthmus --help
Unknown option: '--help'
Usage: isthmus [-m] [--crossjoinpolicy=<crossJoinPolicy>] [-e=<sqlExpression>]
               [--sqlconformancemode=<sqlConformanceMode>]
               [-c=<createStatements>]... [<sql>]
Substrait Java Native Image for parsing SQL Query and SQL Expressions
      [<sql>]            The sql we should parse.
  -c, --create=<createStatements>
                         One or multiple create table statements e.g. CREATE
                           TABLE T1(foo int, bar bigint)
      --crossjoinpolicy=<crossJoinPolicy>
                         One of built-in Calcite SQL compatibility modes:
                           KEEP_AS_CROSS_JOIN, CONVERT_TO_INNER_JOIN
  -e, --sqlExpression=<sqlExpression>
                         The sql expression we should parse.
  -m, --multistatement   Allow multiple statements terminated with a semicolon
      --sqlconformancemode=<sqlConformanceMode>
                         One of built-in Calcite SQL compatibility modes:
                           DEFAULT, LENIENT, BABEL, STRICT_92, STRICT_99,
                           PRAGMATIC_99, BIG_QUERY, MYSQL_5, ORACLE_10,
                           ORACLE_12, STRICT_2003, PRAGMATIC_2003, PRESTO,
                           SQL_SERVER_2008

Extended Expression - Literal Expression

$ ./isthmus -e "3"
{
  "extensionUris": [],
  "extensions": [],
  "referredExpr": [{
    "expression": {
      "literal": {
        "i32": 3,
        "nullable": false,
        "typeVariationReference": 0
      }
    },
    "outputNames": ["new-column"]
  }],
  "baseSchema": {
    "names": [],
    "struct": {
      "types": [],
      "typeVariationReference": 0,
      "nullability": "NULLABILITY_REQUIRED"
    }
  },
  "expectedTypeUrls": []
}

Extended Expression - Referred Expression

$ ./isthmus -c "CREATE TABLE NATION (N_NATIONKEY BIGINT NOT NULL, N_NAME CHAR(25), N_REGIONKEY BIGINT NOT NULL, N_COMMENT VARCHAR(152))" \
    -e  "N_REGIONKEY"
{
  "extensionUris": [],
  "extensions": [],
  "referredExpr": [{
    "expression": {
      "selection": {
        "directReference": {
          "structField": {
            "field": 2
          }
        },
        "rootReference": {
        }
      }
    },
    "outputNames": ["new-column"]
  }],
  "baseSchema": {
    "names": ["N_NATIONKEY", "N_NAME", "N_REGIONKEY", "N_COMMENT"],
    "struct": {
      "types": [{
        "i64": {
          "typeVariationReference": 0,
          "nullability": "NULLABILITY_REQUIRED"
        }
      }, {
        "fixedChar": {
          "length": 25,
          "typeVariationReference": 0,
          "nullability": "NULLABILITY_NULLABLE"
        }
      }, {
        "i64": {
          "typeVariationReference": 0,
          "nullability": "NULLABILITY_REQUIRED"
        }
      }, {
        "varchar": {
          "length": 152,
          "typeVariationReference": 0,
          "nullability": "NULLABILITY_NULLABLE"
        }
      }],
      "typeVariationReference": 0,
      "nullability": "NULLABILITY_REQUIRED"
    }
  },
  "expectedTypeUrls": []
}

Extended Expression - Scalar Function Filter

$ ./isthmus -c "CREATE TABLE NATION (N_NATIONKEY BIGINT NOT NULL, N_NAME CHAR(25), N_REGIONKEY BIGINT NOT NULL, N_COMMENT VARCHAR(152))" \
    -e  "N_REGIONKEY > 10"
{
  "extensionUris": [{
    "extensionUriAnchor": 1,
    "uri": "/functions_comparison.yaml"
  }],
  "extensions": [{
    "extensionFunction": {
      "extensionUriReference": 1,
      "functionAnchor": 0,
      "name": "gt:any_any"
    }
  }],
  "referredExpr": [{
    "expression": {
      "scalarFunction": {
        "functionReference": 0,
        "args": [],
        "outputType": {
          "bool": {
            "typeVariationReference": 0,
            "nullability": "NULLABILITY_REQUIRED"
          }
        },
        "arguments": [{
          "value": {
            "selection": {
              "directReference": {
                "structField": {
                  "field": 2
                }
              },
              "rootReference": {
              }
            }
          }
        }, {
          "value": {
            "cast": {
              "type": {
                "i64": {
                  "typeVariationReference": 0,
                  "nullability": "NULLABILITY_REQUIRED"
                }
              },
              "input": {
                "literal": {
                  "i32": 10,
                  "nullable": false,
                  "typeVariationReference": 0
                }
              },
              "failureBehavior": "FAILURE_BEHAVIOR_UNSPECIFIED"
            }
          }
        }],
        "options": []
      }
    },
    "outputNames": ["new-column"]
  }],
  "baseSchema": {
    "names": ["N_NATIONKEY", "N_NAME", "N_REGIONKEY", "N_COMMENT"],
    "struct": {
      "types": [{
        "i64": {
          "typeVariationReference": 0,
          "nullability": "NULLABILITY_REQUIRED"
        }
      }, {
        "fixedChar": {
          "length": 25,
          "typeVariationReference": 0,
          "nullability": "NULLABILITY_NULLABLE"
        }
      }, {
        "i64": {
          "typeVariationReference": 0,
          "nullability": "NULLABILITY_REQUIRED"
        }
      }, {
        "varchar": {
          "length": 152,
          "typeVariationReference": 0,
          "nullability": "NULLABILITY_NULLABLE"
        }
      }],
      "typeVariationReference": 0,
      "nullability": "NULLABILITY_REQUIRED"
    }
  },
  "expectedTypeUrls": []
}

Extended Expression - Scalar Function Projection

$ ./isthmus -c "CREATE TABLE NATION (N_NATIONKEY BIGINT NOT NULL, N_NAME CHAR(25), N_REGIONKEY BIGINT NOT NULL, N_COMMENT VARCHAR(152))" \
    -e  "N_REGIONKEY + 10"
{
  "extensionUris": [{
    "extensionUriAnchor": 1,
    "uri": "/functions_arithmetic.yaml"
  }],
  "extensions": [{
    "extensionFunction": {
      "extensionUriReference": 1,
      "functionAnchor": 0,
      "name": "add:i64_i64"
    }
  }],
  "referredExpr": [{
    "expression": {
      "scalarFunction": {
        "functionReference": 0,
        "args": [],
        "outputType": {
          "i64": {
            "typeVariationReference": 0,
            "nullability": "NULLABILITY_REQUIRED"
          }
        },
        "arguments": [{
          "value": {
            "selection": {
              "directReference": {
                "structField": {
                  "field": 2
                }
              },
              "rootReference": {
              }
            }
          }
        }, {
          "value": {
            "cast": {
              "type": {
                "i64": {
                  "typeVariationReference": 0,
                  "nullability": "NULLABILITY_REQUIRED"
                }
              },
              "input": {
                "literal": {
                  "i32": 10,
                  "nullable": false,
                  "typeVariationReference": 0
                }
              },
              "failureBehavior": "FAILURE_BEHAVIOR_UNSPECIFIED"
            }
          }
        }],
        "options": []
      }
    },
    "outputNames": ["new-column"]
  }],
  "baseSchema": {
    "names": ["N_NATIONKEY", "N_NAME", "N_REGIONKEY", "N_COMMENT"],
    "struct": {
      "types": [{
        "i64": {
          "typeVariationReference": 0,
          "nullability": "NULLABILITY_REQUIRED"
        }
      }, {
        "fixedChar": {
          "length": 25,
          "typeVariationReference": 0,
          "nullability": "NULLABILITY_NULLABLE"
        }
      }, {
        "i64": {
          "typeVariationReference": 0,
          "nullability": "NULLABILITY_REQUIRED"
        }
      }, {
        "varchar": {
          "length": 152,
          "typeVariationReference": 0,
          "nullability": "NULLABILITY_NULLABLE"
        }
      }],
      "typeVariationReference": 0,
      "nullability": "NULLABILITY_REQUIRED"
    }
  },
  "expectedTypeUrls": []
}

Extended Expression - Scalar Function Concat

$ ./isthmus -c "CREATE TABLE NATION (N_NATIONKEY BIGINT NOT NULL, N_NAME CHAR(25), N_REGIONKEY BIGINT NOT NULL, N_COMMENT VARCHAR(152))" \
    -e  "N_COMMENT || '-' || 'Substrait Java'"
{
  "extensionUris": [{
    "extensionUriAnchor": 1,
    "uri": "/functions_string.yaml"
  }],
  "extensions": [{
    "extensionFunction": {
      "extensionUriReference": 1,
      "functionAnchor": 0,
      "name": "concat:vchar"
    }
  }],
  "referredExpr": [{
    "expression": {
      "scalarFunction": {
        "functionReference": 0,
        "args": [],
        "outputType": {
          "varchar": {
            "length": 167,
            "typeVariationReference": 0,
            "nullability": "NULLABILITY_NULLABLE"
          }
        },
        "arguments": [{
          "value": {
            "scalarFunction": {
              "functionReference": 0,
              "args": [],
              "outputType": {
                "varchar": {
                  "length": 153,
                  "typeVariationReference": 0,
                  "nullability": "NULLABILITY_NULLABLE"
                }
              },
              "arguments": [{
                "value": {
                  "selection": {
                    "directReference": {
                      "structField": {
                        "field": 3
                      }
                    },
                    "rootReference": {
                    }
                  }
                }
              }, {
                "value": {
                  "cast": {
                    "type": {
                      "varchar": {
                        "length": 152,
                        "typeVariationReference": 0,
                        "nullability": "NULLABILITY_NULLABLE"
                      }
                    },
                    "input": {
                      "literal": {
                        "fixedChar": "-",
                        "nullable": false,
                        "typeVariationReference": 0
                      }
                    },
                    "failureBehavior": "FAILURE_BEHAVIOR_UNSPECIFIED"
                  }
                }
              }],
              "options": []
            }
          }
        }, {
          "value": {
            "cast": {
              "type": {
                "varchar": {
                  "length": 153,
                  "typeVariationReference": 0,
                  "nullability": "NULLABILITY_NULLABLE"
                }
              },
              "input": {
                "literal": {
                  "fixedChar": "Substrait Java",
                  "nullable": false,
                  "typeVariationReference": 0
                }
              },
              "failureBehavior": "FAILURE_BEHAVIOR_UNSPECIFIED"
            }
          }
        }],
        "options": []
      }
    },
    "outputNames": ["new-column"]
  }],
  "baseSchema": {
    "names": ["N_NATIONKEY", "N_NAME", "N_REGIONKEY", "N_COMMENT"],
    "struct": {
      "types": [{
        "i64": {
          "typeVariationReference": 0,
          "nullability": "NULLABILITY_REQUIRED"
        }
      }, {
        "fixedChar": {
          "length": 25,
          "typeVariationReference": 0,
          "nullability": "NULLABILITY_NULLABLE"
        }
      }, {
        "i64": {
          "typeVariationReference": 0,
          "nullability": "NULLABILITY_REQUIRED"
        }
      }, {
        "varchar": {
          "length": 152,
          "typeVariationReference": 0,
          "nullability": "NULLABILITY_NULLABLE"
        }
      }],
      "typeVariationReference": 0,
      "nullability": "NULLABILITY_REQUIRED"
    }
  },
  "expectedTypeUrls": []
}

Extended Expression - Scalar Function Like %

$ ./isthmus -c "CREATE TABLE NATION (N_NATIONKEY BIGINT NOT NULL, N_NAME CHAR(25), N_REGIONKEY BIGINT NOT NULL, N_COMMENT VARCHAR(152))" \
    -e  "N_COMMENT like '%PERU'"
{
  "extensionUris": [{
    "extensionUriAnchor": 1,
    "uri": "/functions_string.yaml"
  }],
  "extensions": [{
    "extensionFunction": {
      "extensionUriReference": 1,
      "functionAnchor": 0,
      "name": "like:vchar_vchar"
    }
  }],
  "referredExpr": [{
    "expression": {
      "scalarFunction": {
        "functionReference": 0,
        "args": [],
        "outputType": {
          "bool": {
            "typeVariationReference": 0,
            "nullability": "NULLABILITY_NULLABLE"
          }
        },
        "arguments": [{
          "value": {
            "selection": {
              "directReference": {
                "structField": {
                  "field": 3
                }
              },
              "rootReference": {
              }
            }
          }
        }, {
          "value": {
            "cast": {
              "type": {
                "varchar": {
                  "length": 152,
                  "typeVariationReference": 0,
                  "nullability": "NULLABILITY_NULLABLE"
                }
              },
              "input": {
                "literal": {
                  "fixedChar": "%PERU",
                  "nullable": false,
                  "typeVariationReference": 0
                }
              },
              "failureBehavior": "FAILURE_BEHAVIOR_UNSPECIFIED"
            }
          }
        }],
        "options": []
      }
    },
    "outputNames": ["new-column"]
  }],
  "baseSchema": {
    "names": ["N_NATIONKEY", "N_NAME", "N_REGIONKEY", "N_COMMENT"],
    "struct": {
      "types": [{
        "i64": {
          "typeVariationReference": 0,
          "nullability": "NULLABILITY_REQUIRED"
        }
      }, {
        "fixedChar": {
          "length": 25,
          "typeVariationReference": 0,
          "nullability": "NULLABILITY_NULLABLE"
        }
      }, {
        "i64": {
          "typeVariationReference": 0,
          "nullability": "NULLABILITY_REQUIRED"
        }
      }, {
        "varchar": {
          "length": 152,
          "typeVariationReference": 0,
          "nullability": "NULLABILITY_NULLABLE"
        }
      }],
      "typeVariationReference": 0,
      "nullability": "NULLABILITY_REQUIRED"
    }
  },
  "expectedTypeUrls": []
}

CC @vbarua / @danepitkin / @vibhatha

vibhatha commented 6 months ago

@davisusanibar I tried to use the updated isthmus tool, but I am getting the following error

./isthmus -e "1"
java.lang.ClassCastException: io.substrait.expression.ImmutableExpression$I32Literal cannot be cast to io.substrait.expression.Expression$ScalarFunctionInvocation
    at io.substrait.isthmus.SqlExpressionToSubstrait.executeInnerSQLExpression(SqlExpressionToSubstrait.java:72)
    at io.substrait.isthmus.SqlExpressionToSubstrait.executeSQLExpression(SqlExpressionToSubstrait.java:53)
    at io.substrait.isthmus.IsthmusEntryPoint.call(IsthmusEntryPoint.java:71)
    at io.substrait.isthmus.IsthmusEntryPoint.call(IsthmusEntryPoint.java:18)
    at picocli.CommandLine.executeUserObject(CommandLine.java:1953)
    at picocli.CommandLine.access$1300(CommandLine.java:145)
    at picocli.CommandLine$RunLast.executeUserObjectOfLastSubcommandWithSameParent(CommandLine.java:2352)
    at picocli.CommandLine$RunLast.handle(CommandLine.java:2346)
    at picocli.CommandLine$RunLast.handle(CommandLine.java:2311)
    at picocli.CommandLine$AbstractParseResultHandler.execute(CommandLine.java:2179)
    at picocli.CommandLine.execute(CommandLine.java:2078)
    at io.substrait.isthmus.IsthmusEntryPoint.main(IsthmusEntryPoint.java:58)

Not sure if I am doing something wrong. I have a clean build though :thinking:

vibhatha commented 6 months ago

Also can we do something like

/isthmus -c "CREATE TABLE NATION (N_NATIONKEY BIGINT NOT NULL, N_NAME CHAR(25), N_REGIONKEY BIGINT NOT NULL, N_COMMENT VARCHAR(152))" \
    -e  "N_REGIONKEY > 10 && N_REGIONKEY < 100"
davisusanibar commented 5 months ago

@davisusanibar I tried to use the updated isthmus tool, but I am getting the following error

./isthmus -e "1"
java.lang.ClassCastException: io.substrait.expression.ImmutableExpression$I32Literal cannot be cast to io.substrait.expression.Expression$ScalarFunctionInvocation
  at io.substrait.isthmus.SqlExpressionToSubstrait.executeInnerSQLExpression(SqlExpressionToSubstrait.java:72)
  at io.substrait.isthmus.SqlExpressionToSubstrait.executeSQLExpression(SqlExpressionToSubstrait.java:53)
  at io.substrait.isthmus.IsthmusEntryPoint.call(IsthmusEntryPoint.java:71)
  at io.substrait.isthmus.IsthmusEntryPoint.call(IsthmusEntryPoint.java:18)
  at picocli.CommandLine.executeUserObject(CommandLine.java:1953)
  at picocli.CommandLine.access$1300(CommandLine.java:145)
  at picocli.CommandLine$RunLast.executeUserObjectOfLastSubcommandWithSameParent(CommandLine.java:2352)
  at picocli.CommandLine$RunLast.handle(CommandLine.java:2346)
  at picocli.CommandLine$RunLast.handle(CommandLine.java:2311)
  at picocli.CommandLine$AbstractParseResultHandler.execute(CommandLine.java:2179)
  at picocli.CommandLine.execute(CommandLine.java:2078)
  at io.substrait.isthmus.IsthmusEntryPoint.main(IsthmusEntryPoint.java:58)

Not sure if I am doing something wrong. I have a clean build though 🤔

Let me review this error message.

vibhatha commented 5 months ago

I got the same error when I checked out and built locally.

vibhatha commented 5 months ago

I think the explicit cast used in the code is the issue, can we avoid it with reflection?

davisusanibar commented 5 months ago
./isthmus -e "1"

It is working now:

./gradlew nativeImage 
./isthmus/build/graal/isthmus -e "1" --> IllegalArgumentException: Information regarding the data and types must be passed. 
./isthmus/build/graal/isthmus -e "1" --create "create table dummy(a int)"
{
  "extensionUris": [],
  "extensions": [],
  "referredExpr": [{
    "expression": {
      "literal": {
        "i32": 1,
        "nullable": false,
        "typeVariationReference": 0
      }
    },
    "outputNames": ["new-column"]
  }],
  "baseSchema": {
    "names": ["A"],
    "struct": {
      "types": [{
        "i32": {
          "typeVariationReference": 0,
          "nullability": "NULLABILITY_NULLABLE"
        }
      }],
      "typeVariationReference": 0,
      "nullability": "NULLABILITY_REQUIRED"
    }
  },
  "expectedTypeUrls": []
}
{
  "extensionUris": [],
  "extensions": [],
  "referredExpr": [{
    "expression": {
      "literal": {
        "i32": 1,
        "nullable": false,
        "typeVariationReference": 0
      }
    },
    "outputNames": ["new-column"]
  }],
  "baseSchema": {
    "names": ["A"],
    "struct": {
      "types": [{
        "i32": {
          "typeVariationReference": 0,
          "nullability": "NULLABILITY_NULLABLE"
        }
      }],
      "typeVariationReference": 0,
      "nullability": "NULLABILITY_REQUIRED"
    }
  },
  "expectedTypeUrls": []
}

I need to fix error related to IllegalArgumentException: Information regarding the data and types must be passed when no data type information is needed to provide.

davisusanibar commented 5 months ago

Also can we do something like

/isthmus -c "CREATE TABLE NATION (N_NATIONKEY BIGINT NOT NULL, N_NAME CHAR(25), N_REGIONKEY BIGINT NOT NULL, N_COMMENT VARCHAR(152))" \
    -e  "N_REGIONKEY > 10 && N_REGIONKEY < 100"

Just able to resolve (it is pending to validate if it resolve ok with a parquet file for example):


./isthmus/build/graal/isthmus -c "CREATE TABLE NATION (N_NATIONKEY BIGINT NOT NULL, N_NAME CHAR(25), N_REGIONKEY BIGINT NOT NULL, N_COMMENT VARCHAR(152))" \
    -e  "N_REGIONKEY > 10 AND N_REGIONKEY < 100"

{
  "extensionUris": [
    {
      "extensionUriAnchor": 1,
      "uri": "/functions_boolean.yaml"
    },
    {
      "extensionUriAnchor": 2,
      "uri": "/functions_comparison.yaml"
    }
  ],
  "extensions": [
    {
      "extensionFunction": {
        "extensionUriReference": 1,
        "functionAnchor": 0,
        "name": "and:bool"
      }
    },
    {
      "extensionFunction": {
        "extensionUriReference": 2,
        "functionAnchor": 1,
        "name": "gt:any_any"
      }
    },
    {
      "extensionFunction": {
        "extensionUriReference": 2,
        "functionAnchor": 2,
        "name": "lt:any_any"
      }
    }
  ],
  "referredExpr": [
    {
      "expression": {
        "scalarFunction": {
          "functionReference": 0,
          "args": [],
          "outputType": {
            "bool": {
              "typeVariationReference": 0,
              "nullability": "NULLABILITY_REQUIRED"
            }
          },
          "arguments": [
            {
              "value": {
                "scalarFunction": {
                  "functionReference": 1,
                  "args": [],
                  "outputType": {
                    "bool": {
                      "typeVariationReference": 0,
                      "nullability": "NULLABILITY_REQUIRED"
                    }
                  },
                  "arguments": [
                    {
                      "value": {
                        "selection": {
                          "directReference": {
                            "structField": {
                              "field": 2
                            }
                          },
                          "rootReference": {}
                        }
                      }
                    },
                    {
                      "value": {
                        "cast": {
                          "type": {
                            "i64": {
                              "typeVariationReference": 0,
                              "nullability": "NULLABILITY_REQUIRED"
                            }
                          },
                          "input": {
                            "literal": {
                              "i32": 10,
                              "nullable": false,
                              "typeVariationReference": 0
                            }
                          },
                          "failureBehavior": "FAILURE_BEHAVIOR_UNSPECIFIED"
                        }
                      }
                    }
                  ],
                  "options": []
                }
              }
            },
            {
              "value": {
                "scalarFunction": {
                  "functionReference": 2,
                  "args": [],
                  "outputType": {
                    "bool": {
                      "typeVariationReference": 0,
                      "nullability": "NULLABILITY_REQUIRED"
                    }
                  },
                  "arguments": [
                    {
                      "value": {
                        "selection": {
                          "directReference": {
                            "structField": {
                              "field": 2
                            }
                          },
                          "rootReference": {}
                        }
                      }
                    },
                    {
                      "value": {
                        "cast": {
                          "type": {
                            "i64": {
                              "typeVariationReference": 0,
                              "nullability": "NULLABILITY_REQUIRED"
                            }
                          },
                          "input": {
                            "literal": {
                              "i32": 100,
                              "nullable": false,
                              "typeVariationReference": 0
                            }
                          },
                          "failureBehavior": "FAILURE_BEHAVIOR_UNSPECIFIED"
                        }
                      }
                    }
                  ],
                  "options": []
                }
              }
            }
          ],
          "options": []
        }
      },
      "outputNames": [
        "new-column"
      ]
    }
  ],
  "baseSchema": {
    "names": [
      "N_NATIONKEY",
      "N_NAME",
      "N_REGIONKEY",
      "N_COMMENT"
    ],
    "struct": {
      "types": [
        {
          "i64": {
            "typeVariationReference": 0,
            "nullability": "NULLABILITY_REQUIRED"
          }
        },
        {
          "fixedChar": {
            "length": 25,
            "typeVariationReference": 0,
            "nullability": "NULLABILITY_NULLABLE"
          }
        },
        {
          "i64": {
            "typeVariationReference": 0,
            "nullability": "NULLABILITY_REQUIRED"
          }
        },
        {
          "varchar": {
            "length": 152,
            "typeVariationReference": 0,
            "nullability": "NULLABILITY_NULLABLE"
          }
        }
      ],
      "typeVariationReference": 0,
      "nullability": "NULLABILITY_REQUIRED"
    }
  },
  "expectedTypeUrls": []
}
vibhatha commented 5 months ago

@davisusanibar what was the change?

davisusanibar commented 5 months ago

@davisusanibar what was the change?

Hi Vibhatha, I have only changed && to AND. In addition, the PR has been merged with the latest changes.

davisusanibar commented 5 months ago
./isthmus -e "1"

It is working now:

./gradlew nativeImage 
./isthmus/build/graal/isthmus -e "1" --> IllegalArgumentException: Information regarding the data and types must be passed. 
./isthmus/build/graal/isthmus -e "1" --create "create table dummy(a int)"
{
  "extensionUris": [],
  "extensions": [],
  "referredExpr": [{
    "expression": {
      "literal": {
        "i32": 1,
        "nullable": false,
        "typeVariationReference": 0
      }
    },
    "outputNames": ["new-column"]
  }],
  "baseSchema": {
    "names": ["A"],
    "struct": {
      "types": [{
        "i32": {
          "typeVariationReference": 0,
          "nullability": "NULLABILITY_NULLABLE"
        }
      }],
      "typeVariationReference": 0,
      "nullability": "NULLABILITY_REQUIRED"
    }
  },
  "expectedTypeUrls": []
}
{
  "extensionUris": [],
  "extensions": [],
  "referredExpr": [{
    "expression": {
      "literal": {
        "i32": 1,
        "nullable": false,
        "typeVariationReference": 0
      }
    },
    "outputNames": ["new-column"]
  }],
  "baseSchema": {
    "names": ["A"],
    "struct": {
      "types": [{
        "i32": {
          "typeVariationReference": 0,
          "nullability": "NULLABILITY_NULLABLE"
        }
      }],
      "typeVariationReference": 0,
      "nullability": "NULLABILITY_REQUIRED"
    }
  },
  "expectedTypeUrls": []
}

I need to fix error related to IllegalArgumentException: Information regarding the data and types must be passed when no data type information is needed to provide.

A fix has been applied to support literal expressions.

davisusanibar commented 5 months ago

@vibhatha / @vbarua / @danepitkin I would greatly appreciate your feedback.

vibhatha commented 5 months ago

@davisusanibar one issue here: https://github.com/substrait-io/substrait-java/actions/runs/7614170736/job/20735814913?pr=209#step:5:75

davisusanibar commented 5 months ago

I am reviewing error message related to:

Error: Execution failed for task ':core:compileTestJava':
      org.gradle.api.internal.tasks.compile.CompilationFailedException: Compilation failed; see the compiler error output for details.
          at org.gradle.api.internal.tasks.compile.JdkJavaCompiler.execute(JdkJavaCompiler.java:57)
...
/home/runner/work/substrait-java/substrait-java/core/src/test/java/io/substrait/type/proto/LocalFilesRoundtripTest.java:61: error: switch expressions are not supported in -source 8
    return switch (pathTypeCase) {
           ^
  (use -source 14 or higher to enable switch expressions)
/home/runner/work/substrait-java/substrait-java/core/src/test/java/io/substrait/type/proto/LocalFilesRoundtripTest.java:62: error: switch rules are not supported in -source 8
      case URI_PATH -> builder.pathType(FileOrFiles.PathType.URI_PATH).path("path");
                    ^
  (use -source 14 or higher to enable switch rules)
vibhatha commented 5 months ago

I am reviewing error message related to:

Error: Execution failed for task ':core:compileTestJava':
      org.gradle.api.internal.tasks.compile.CompilationFailedException: Compilation failed; see the compiler error output for details.
          at org.gradle.api.internal.tasks.compile.JdkJavaCompiler.execute(JdkJavaCompiler.java:57)
...
/home/runner/work/substrait-java/substrait-java/core/src/test/java/io/substrait/type/proto/LocalFilesRoundtripTest.java:61: error: switch expressions are not supported in -source 8
    return switch (pathTypeCase) {
           ^
  (use -source 14 or higher to enable switch expressions)
/home/runner/work/substrait-java/substrait-java/core/src/test/java/io/substrait/type/proto/LocalFilesRoundtripTest.java:62: error: switch rules are not supported in -source 8
      case URI_PATH -> builder.pathType(FileOrFiles.PathType.URI_PATH).path("path");
                    ^
  (use -source 14 or higher to enable switch rules)

@davisusanibar are you running from the IDE?

vibhatha commented 5 months ago

@davisusanibar nevermind, I got the context. I guess this is coming from the syntactic sugar library we are using to get higher JDK features? Could we avoid using it here? In case if this not properly supported in the libraries we are using? Or should we re-run these CIs and see if this is reproducible?

davisusanibar commented 5 months ago

@davisusanibar nevermind, I got the context. I guess this is coming from the syntactic sugar library we are using to get higher JDK features? Could we avoid using it here? In case if this not properly supported in the libraries we are using? Or should we re-run these CIs and see if this is reproducible?

There is an error only at GithubCI, I'm reviewing these options:

davisusanibar commented 5 months ago

It was not possible for me to reproduce the CI error locally.

Finally, I updated https://github.com/gradle/gradle-build-action from v2 to v3 and it now works perfectly.

I would appreciate if you could continue your review @danepitkin @vibhatha @vbarua

vibhatha commented 5 months ago

@davisusanibar nit: should we use System.out. or logger.info

davisusanibar commented 5 months ago

Nice work! Just one more thing:

Can we update the Isthmus README with the new cli arg? https://github.com/substrait-io/substrait-java/blob/main/isthmus/README.md

Just updated, and also added a support for --help and --version options.

davisusanibar commented 5 months ago

@davisusanibar nit: should we use System.out. or logger.info

I deleted log options and added logic to handle:

davisusanibar commented 5 months ago

A new issue has been created to evaluate the possibility of synchronizing the current fixed value version with the semantic release version: https://github.com/substrait-io/substrait-java/issues/223

davisusanibar commented 5 months ago

@vibhatha, would it be possible for you to review this again?

A change was just made to allow passing lists of expressions and expression sequences by separator (comma by default).

vbarua commented 5 months ago

@davisusanibar I had some small feedback which I decided to turn into a commit. If it looks good to you, I can squash and merge this tomorrow.

https://github.com/substrait-io/substrait-java/pull/209/commits/908d5e322ff7e718bae12c4d2dca7de4d652e425

davisusanibar commented 5 months ago

@davisusanibar I had some small feedback which I decided to turn into a commit. If it looks good to you, I can squash and merge this tomorrow.

908d5e3

Thanks a lot for this kind of refinement all the time. I appreciate your assistance.