[CALCITE-5052] Allow resources created with a JAR protocol (Bazel) #2750

mkou · 2022-03-22T16:22:28Z

No description provided.

This allows dependent projects to run tests using Bazel. (Previously, DiffRepository would give errors because Bazel has packaged the .xml files it needs inside JAR files.) Close apache#2750

…TE-5155) (#9) * [CALCITE-5197] Bump gradle to 7.4.2 Add a Gradle task to automatically update the checksum in the Gradle Wrapper. In HOWTO guide, add a section 'How to ugrade Gradle and the Gradle Wrapper', and update the Gradle version. Close apache#2841 * [CALCITE-5344] Migrate Travis CI and AppVeyor configuration to Github Actions The ASF is discontinuing Travis CI for testing, and it will no longer be available after 31 December 2022. * [CALCITE-5340] Tests should fail when actual and expected XML reference files are not identical File check added to the following test suites: - HepPlannerTest - RelOptRulesTest - RuleMatchVisualizerTest - SqlHintsConverterTest - SqlToRelConverterTest - SqlPrettyWriterTest - TypeCoercionConverterTest - TopDownOptTest Updated XML test reference files: - HepPlannerTest.xml - RelOptRulesTest.xml - SqlHintsConverterTest.xml - SqlToRelConverterTest.xml - TypeCoercionConverterTest.xml * [CALCITE-5314] Prune empty parts of a query by exploiting stats/metadata Close apache#2935 * [CALCITE-4804] Support Snapshot operator serialization and deserizalization This closes apache#2955 * [CALCITE-5351] Upgrade jackson databind to 2.13.4.2 and jackson to 2.13.4 * [CALCITE-5355] Use the Presto SQL dialect for AWS Athena * [CALCITE-5252] JDBC adapter sometimes misses parentheses around Query in WITH_ITEM body This closes apache#2952 * [CALCITE-4982] Do not push 'cast to not null' through Join in ProjectJoinTransposeRule We should not push down dangerous expressions through Join at last, see more in CALCITE-5315. In this issue, we fixed this by stop pushing down 'cast to not null' through Join for a quick fix. This closes 2686 * Use JDK 17 as default javadoc root * Refactor tests to allow testing custom type systems * Refactor RexImpTable Break up the constructor into a builder pattern, and make RexImpTable immutable after construction. The constructor was getting too long (approaching 400 lines), so this change introduces a private static class Builder and moves initialization into chained populate() and populate2() methods. Usually we put inner classes towards the end of the file. In this case, we put the Builder inner class after the constructor, in order to reduce diff noise. * Refactor: Deprecate SqlValidatorUtil.getAlias Whether `SqlValidatorUtil.getAlias` returns null depends on whether the ordinal argument is less than zero, which makes it difficult to reason about nullability. Replace with two methods `alias(SqlNode)` that is nullable and `alias(SqlNode, int)` that is not nullable. * Various improvements to OperandTypes In SqlSingleOperandTypeChecker.java add default method implementations so that subtypes don't need to. Add OperandTypes.interval(SqlTypeName) to match an interval operand. Add CompositeOperandTypeChecker.withGenerator, so that a checker can generate signatures given a function name, as opposed to hard-coding the function name in the signatures as today. Add SqlOperandTypeChecker.and, SqlSingleOperandTypeChecker.or, and similar methods, to combine checkers using a binary operation. Flatten composite checkers, so that 'or(or(a, b), c)' becomes 'or(a, b,c)'. * Remove Nullable from RelBuilder.alias * Refactor SqlValidatorNamespace.fieldExists Add method SqlValidatorNamespace.field * Improve digest for Window * [CALCITE-5348] When translating ORDER BY in OVER, use the session's default null collation (e.g. NULLS LAST) This repeats the fix that was made in [CALCITE-2323] and that I accidentally undid in [CALCITE-4497]. Deprecate method SqlToRelConverter.convertSortExpression; makeOver now goes directly to RexNode rather than using an intermediate RexFieldCollation. * Add class MonotonicSupplier * Add "mssql" (Microsoft SQL Server) function library Add functions DATEADD, DATEDIFF, DATEPART, DATE_PART as aliases of TIMESTAMPADD, TIMESTAMPDIFF, EXTRACT. (Not fully implemented yet.) * [CALCITE-5372] Upgrade vlsi-release-plugins to 1.84 * [CALCITE-5155] Custom time frames Before this change, you can use the ISO SQL time units (SECOND, HOUR, DAY, MONTH, YEAR, etc.) to perform datetime arithmetic (FLOOR, CEIL, EXTRACT) and also when defining materialized views and using them in queries. But applications would like to be able to define their own time frames, such as "MINUTE15" (a 15 minute period aligned with the hour) or "MONTH4" (a 4 month period aligned with the year), or "WEEK(THURSDAY)" (a 7 day week that starts on a Thursday). After this change, applications can define their own time frames. We add a `class TimeFrameSet`, and in the `interface RelDataTypeSystem` we add a method `TimeFrameSet deriveTimeFrameSet(TimeFrameSet frameSet)`. This method is called during query preparation, and the application has the opportunity to define a set that contains custom and existing time frames. Time frames can be defined that are multiples of and multiply to built-in time frames (as, for example, MINUTE is a multiple of SECOND and MILLISECOND multiplies to SECOND). You can also define that a time frame is aligned with another (as, for example, DAY is aligned with MONTH even though the multiplier is not constant). The following functions allow time frame arguments: * DATEADD (Postgres, MSSql) * DATEDIFF (Postgres, MSSql) * DATEPART (MSSql) * DATE_PART (Postgres) * EXTRACT (Calcite built-in, also SQL standard) * CEIL (Calcite built-in) * FLOOR (Calcite built-in) * TIMESTAMPADD (Calcite built-in, also JDBC standard) * TIMESTAMPDIFF (Calcite-builtin, also JDBC standard) * TIMESTAMP_TRUNC (BigQuery) * TIME_TRUNC (BigQuery) Calls to the above functions with invalid time units would previously be a parse error and are now detected during validation. The SQL_TSI_xxx (e.g. SQL_TSI_HOUR) arguments are treated as time frames, and the parser passes them as identifiers. They are no longer reserved keywords. Previously, NANOSECOND and MILLISECOND were allowed in EXTRACT but no other functions. Now all functions that accept time frames accept the same time frames (built-in time intervals, identifiers for user-defined time frames, and SQL_TSI_xxx which are defined in the JDBC standard but are treated as identifiers until validation). The representation of calls to the above functions has changed. Previously the operand was a time unit, now it is an identifier. Deprecate SqlAbstractParserImpl.setTimeUnitCodes() and SqlParser.Config.timeUnitCodes(), because you can now create aliases for time units by creating custom time frames using TimeFrameSet.Builder.alias(). Add commons-math3 as a dependency because TimeFrame uses BigFraction. Currently ISO_YEAR is not handled by DATEADD, TIMESTAMPADD, DATEDIFF, TIMESTAMPDIFF, etc. Adding or subtracting an ISO_YEAR will no-op. I don't know what the behavior should be. Close apache#2960 * [CALCITE-5356] Update junit4 to 4.13.2 and junit5 to 5.9.1 Close apache#2958 * Site: Add instructions to consult/update the JIRA release dashboard Close apache#2963 * Site: Add Dmitry Sysolyatin as committer * Site: Add Bertil Chapuis as committer * [CALCITE-5353] Document new procedure for requesting JIRA accounts and becoming a contributor * [CALCITE-5383] Add CONCAT to BIG_QUERY dialect Close apache#2970 * [CALCITE-5310] JSON_OBJECT in scalar sub-query throws AssertionError Close apache#2929 * Quidem: Allow CREATE VIEW in 'scott' connection * [CALCITE-5105] Add MEASURE type and AGGREGATE aggregate function The MEASURE type is internal. A RexNode expression that contains a measure and evaluates to an INTEGER will have the type MEASURE<INTEGER>, using a parameterized SQL type similar to ARRAY<INTEGER>. But the same measure column, as seen from SQL, will just have type INTEGER. The parameterized type helps us keep things straight in the relational algebra as we apply planner rules. The AGGREGATE function belongs to the new CALCITE function library. To use it, add 'lib=calcite' to your connect string. Add a new validator configuration parameter, `boolean SqlValidator.Config.nakedMeasures()`. The query SELECT deptno, AGGREGATE(avg_sal) FROM emp GROUP BY deptno is valid if `avg_sal` is a measure. If `nakedMeasures` is true, then the following query is a valid shorthand for it: SELECT deptno, avg_sal FROM emp GROUP BY deptno In the long term, we would like people to feel comfortable using the latter form. Measures are not necessarily aggregate functions, but are just expressions whose value depends on their context (the current GROUP BY key in an aggregate query, or the current row in a regular query). And we will generalize measures to analytic expressions, which are not necessarily just references to measure columns. But in the short term, setting the `nakedMeasures` flag to false provides a level of comfort to people (and tools that generate SQL) who think of measures as aggregate functions, and think that measures should only be used in `GROUP BY` queries. Extend mock catalog with a table that has measure columns. Add a new Quidem test, measure.iq. It is disabled because we don't yet have the means to create measure columns in queries (or views). That is to come in [CALCITE-4496]. Close apache#2965 * Workaround * Looker instructions * Temporary workaround: change SqlInternalOperator's syntax back to FUNCTION * [CALCITE-4998] Undo 4b34903 * Make sure the test kit can be used externally * [CALCITE-5052] Allow Source based on a URL with jar: protocol This allows dependent projects to run tests using Bazel. (Previously, DiffRepository would give errors because Bazel has packaged the .xml files it needs inside JAR files.) Close apache#2750 * [CALCITE-5349] RelJson deserialization should support SqlLibraryOperators * Support UDT declarations from root of schema model. (#7) Related to CALCITE-5346. Previously we relied on the parser to map unrecognized datatypes to a known type using SqlAlienSystemTypeNameSpec. This worked but made it difficult to change or add new types as necessary. One would have to update at least 3 different parsers (babel, core, server) to make a change. This change allows for declaring user-defined types at the root of a schema model and allows for easy type alias mapping. These data types are shared by all schema in the model so cast and DDL expressions do not need to scope data type references to a particular sub-schema. For example: ``` inline: { version: '1.0', types: [ { name: 'BOOL', type: 'BOOLEAN' }, { name: 'BYTES', type: 'VARBINARY' }, ... ], ``` Allows for `CAST("true" as BOOL)` * Rebase and clean up git history Co-authored-by: Sergey Nuyanzin <[email protected]> Co-authored-by: Francis Chuang <[email protected]> Co-authored-by: Alessandro Solimando <[email protected]> Co-authored-by: Hanumath Maduri <[email protected]> Co-authored-by: xiejiajun <[email protected]> Co-authored-by: James Turton <[email protected]> Co-authored-by: wumou.wm <[email protected]> Co-authored-by: xurenhe <[email protected]> Co-authored-by: Julian Hyde <[email protected]> Co-authored-by: Stamatis Zampetakis <[email protected]> Co-authored-by: dssysolyatin <[email protected]> Co-authored-by: Bertil Chapuis <[email protected]> Co-authored-by: Oliver Lee <[email protected]> Co-authored-by: Benchao Li <[email protected]> Co-authored-by: Will Noble <[email protected]> Co-authored-by: Marieke Gueye <[email protected]> Co-authored-by: TJ Banghart <[email protected]>

This allows dependent projects to run tests using Bazel. (Previously, DiffRepository would give errors because Bazel has packaged the .xml files it needs inside JAR files.) Close apache#2750

tanclary

Probably worth checking how much the test fixtures have changed since this was last update (maybe some or maybe none at all)

tanclary · 2023-12-27T22:44:51Z

core/src/test/java/org/apache/calcite/util/SourceTest.java

@@ -121,6 +131,54 @@ void testAbsoluteFileToUrl(String path, String expectedUrl) throws URISyntaxExce
        () -> "Sources.of(Sources.of(file(" + path + ").absolutePath).url()).file().getPath()");
  }

+  /**
+   * Resources created with the JAR protocol should also work.


nit could reference JIRA here and in below test.

tanclary · 2023-12-27T22:48:33Z

core/src/test/java/org/apache/calcite/util/SourceTest.java

+  /** Tests {@link Sources#of(URL)} with code similar to
+   * {@code DiffRepository.Key#toRepo()}. */
+  @Test void testJarFileUrlWrite() {
+    final URL refFile = FoodmartQuerySet.class.getResource("/queries.json");


To be honest I'm a bit confused at what all is going on here. Would you mind either:

Extracting some of the setup into a helper (maybe easier testing is possible since this PR was opened).

Breaking this into multiple tests

Adding more comments

If it's all common sense and I'm OOTL then disregard.

Crap it is so long ago I don't remember why I did that to test the diff repo

testkit/src/main/java/org/apache/calcite/test/DiffRepository.java

tanclary · 2024-01-04T23:02:27Z

core/src/test/java/org/apache/calcite/util/SourceTest.java

@@ -40,7 +42,7 @@

 import static org.hamcrest.CoreMatchers.is;
 import static org.hamcrest.MatcherAssert.assertThat;
-import static org.hamcrest.Matchers.hasToString;
+import static org.hamcrest.Matchers.*;


I think Calcite uses explicit imports rather than .*; this is configurable in intellij settings

https://www.jetbrains.com/help/idea/creating-and-optimizing-imports.html#disable-wildcard-imports

tanclary · 2024-01-04T23:04:22Z

core/src/test/java/org/apache/calcite/util/SourceTest.java

@@ -118,6 +120,22 @@ void testAbsoluteFileToUrl(String path, String expectedUrl) throws URISyntaxExce
        () -> "Sources.of(Sources.of(file(" + path + ").absolutePath).url()).file().getPath()");
  }

+  /**
+   * [CALCITE-5052] Resources created with the JAR protocol should also work.


if youre going to link a jira case you can format it like this example: https://github.com/apache/calcite/blob/main/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java#L277

tanclary · 2024-01-04T23:04:43Z

testkit/src/main/java/org/apache/calcite/test/DiffRepository.java

@@ -877,7 +880,7 @@ public static DiffRepository lookup(Class<?> clazz,
    return REPOSITORY_CACHE.getUnchecked(key);
  }

-  /**
+    /**


tanclary · 2024-01-04T23:05:22Z

testkit/src/main/java/org/apache/calcite/test/DiffRepository.java

    private final Class<?> clazz;
    private final DiffRepository baseRepository;
    private final Filter filter;
    private final int indent;
-
+    private URL refFile;
+    String refFilePath;


To be honest I like the newline after all the variables and before the Key() {}

tanclary · 2024-01-04T23:09:53Z

testkit/src/test/java/org/apache/calcite/test/DiffRepositoryTest.java

+   * would create a log file outside of the JAR.
+   **/
+  @Test void testLogFilePathInJar() {
+    final DiffRepository diffRepo = JarStubDiffRepository.lookup(FixtureTest.class);


Looks so much better now

tanclary · 2024-01-04T23:11:22Z

testkit/src/test/java/org/apache/calcite/test/DiffRepositoryTest.java

+  }
+
+  public static DiffRepository lookup(Class<?> clazz) {
+    final JarKey key = new JarKey(clazz, null, null, 2);


should these have comments // be spaced out ? I think the actual logic looks fine !

This allows dependent projects to run tests using Bazel. (Previously, DiffRepository would give errors because Bazel has packaged the .xml files it needs inside JAR files.) Close apache#2750

mkou mentioned this pull request Mar 22, 2022

[CALCITE-5052] Allow resources created with a JAR protocol (Bazel) #2748

Closed

julianhyde force-pushed the main branch from fa65a2e to 1226d1a Compare June 20, 2022 20:27

asfgit force-pushed the main branch from 9fc50f2 to e2f949d Compare September 10, 2022 16:37

asfgit force-pushed the main branch from f8f8a51 to a326bd2 Compare January 25, 2023 07:39

julianhyde force-pushed the main branch 2 times, most recently from 8a5cf83 to cf7f71b Compare June 8, 2023 21:21

libenchao force-pushed the main branch from 47db81a to 0be8eae Compare November 10, 2023 13:18

tanclary reviewed Dec 27, 2023

View reviewed changes

[CALCITE-5052] Fixtures: allow resources created with a JAR protocol

292489d

mkou force-pushed the CALCITE-5052-allow-jar-resources- branch from 7e5a0cb to 292489d Compare January 4, 2024 22:40

autostyle

1a92745

tanclary requested changes Jan 4, 2024

View reviewed changes

F21 force-pushed the main branch from 7d38212 to cacf36a Compare February 17, 2025 03:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[CALCITE-5052] Allow resources created with a JAR protocol (Bazel) #2750

[CALCITE-5052] Allow resources created with a JAR protocol (Bazel) #2750

mkou commented Mar 22, 2022

tanclary left a comment

tanclary Dec 27, 2023

tanclary Dec 27, 2023

mkou Dec 28, 2023

tanclary Jan 4, 2024

tanclary Jan 4, 2024

tanclary Jan 4, 2024

tanclary Jan 4, 2024

tanclary Jan 4, 2024

tanclary Jan 4, 2024

[CALCITE-5052] Allow resources created with a JAR protocol (Bazel) #2750

Are you sure you want to change the base?

[CALCITE-5052] Allow resources created with a JAR protocol (Bazel) #2750

Conversation

mkou commented Mar 22, 2022

tanclary left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment