You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by jh...@apache.org on 2023/06/29 00:17:36 UTC
[calcite] branch main updated (50f0e18589 -> 7bb4f4191a)
This is an automated email from the ASF dual-hosted git repository.
jhyde pushed a change to branch main
in repository https://gitbox.apache.org/repos/asf/calcite.git
from 50f0e18589 Site: Add Yong Liu as committer
new d4276bcc19 [CALCITE-5786] QuidemTest and DiffRepository are not compatible with Gradle incremental builds since they write to build/resources
new 7bb4f4191a [CALCITE-5793] JDBC adapter should use NULLS FIRST, NULLS LAST syntax for BigQuery
The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
.../calcite/sql/dialect/BigQuerySqlDialect.java | 5 ---
.../calcite/rel/rel2sql/RelToSqlConverterTest.java | 4 +-
.../org/apache/calcite/test/RelOptRulesTest.java | 10 +++--
.../org/apache/calcite/test/DiffRepository.java | 20 ++++++----
.../java/org/apache/calcite/test/QuidemTest.java | 44 ++++++++++++++--------
5 files changed, 49 insertions(+), 34 deletions(-)
[calcite] 02/02: [CALCITE-5793] JDBC adapter should use NULLS FIRST, NULLS LAST syntax for BigQuery
Posted by jh...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
jhyde pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/calcite.git
commit 7bb4f4191ae9f91619d645bc51d6433eebe4d530
Author: Will Noble <wn...@google.com>
AuthorDate: Tue Jun 20 10:54:50 2023 -0700
[CALCITE-5793] JDBC adapter should use NULLS FIRST, NULLS LAST syntax for BigQuery
BigQuery has supported NULLS FIRST, NULLS LAST since 2020.
It makes sense to use it, because the previous emulation,
e.g. 'ORDER BY x IS NULL, x', would cause problems such as
[CALCITE-5775] if x was a complex expression.
Close apache/calcite#3276
---
.../main/java/org/apache/calcite/sql/dialect/BigQuerySqlDialect.java | 5 -----
.../java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java | 4 ++--
2 files changed, 2 insertions(+), 7 deletions(-)
diff --git a/core/src/main/java/org/apache/calcite/sql/dialect/BigQuerySqlDialect.java b/core/src/main/java/org/apache/calcite/sql/dialect/BigQuerySqlDialect.java
index e7a91dea2e..15ae0bad0b 100644
--- a/core/src/main/java/org/apache/calcite/sql/dialect/BigQuerySqlDialect.java
+++ b/core/src/main/java/org/apache/calcite/sql/dialect/BigQuerySqlDialect.java
@@ -108,11 +108,6 @@ public class BigQuerySqlDialect extends SqlDialect {
|| RESERVED_KEYWORDS.contains(val.toUpperCase(Locale.ROOT));
}
- @Override public @Nullable SqlNode emulateNullDirection(SqlNode node,
- boolean nullsFirst, boolean desc) {
- return emulateNullDirectionWithIsNull(node, nullsFirst, desc);
- }
-
@Override public boolean supportsImplicitTypeCoercion(RexCall call) {
return super.supportsImplicitTypeCoercion(call)
&& RexUtil.isLiteral(call.getOperands().get(0), false)
diff --git a/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java b/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java
index 45997d6cfd..2904e144ac 100644
--- a/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java
+++ b/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java
@@ -3180,7 +3180,7 @@ class RelToSqlConverterTest {
// BigQuery uses LIMIT/OFFSET, and nulls sort low by default
final String expectedBigQuery = "SELECT product_id\n"
+ "FROM foodmart.product\n"
- + "ORDER BY net_weight IS NULL, net_weight\n"
+ + "ORDER BY net_weight NULLS LAST\n"
+ "LIMIT 100\n"
+ "OFFSET 10";
sql(query).ok(expected)
@@ -7074,7 +7074,7 @@ class RelToSqlConverterTest {
+ "FROM SCOTT.EMP\n"
+ "GROUP BY DEPTNO\n"
+ "HAVING COUNT(DISTINCT EMPNO) > 0\n"
- + "ORDER BY COUNT(DISTINCT EMPNO) IS NULL DESC, 2 DESC";
+ + "ORDER BY 2 DESC NULLS FIRST";
// Convert rel node to SQL with BigQuery dialect,
// in which "isHavingAlias" is true.
[calcite] 01/02: [CALCITE-5786] QuidemTest and DiffRepository are not compatible with Gradle incremental builds since they write to build/resources
Posted by jh...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
jhyde pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/calcite.git
commit d4276bcc19523285ed9385a52741d15e16315e78
Author: Akshay Dayal <ad...@gradle.com>
AuthorDate: Mon Jun 26 02:30:26 2023 -0600
[CALCITE-5786] QuidemTest and DiffRepository are not compatible with Gradle incremental builds since they write to build/resources
This commit solves the problem by writing to quidem and diffrepo directories rather than resources.
QuidemTest for
src/test/resources/foo.iq
now writes to
build/quidem/test/foo.iq
rather than
build/resources/test/surefire/foo.iq
DiffRepository for
src/test/resources/com/example/FooTest.xml
now writes to
build/diffrepo/test/com/example/FooTest_actual.xml
rather than
build/resources/com/example/Foo_actual.xml
Close apache/calcite#3280
---
.../org/apache/calcite/test/RelOptRulesTest.java | 10 +++--
.../org/apache/calcite/test/DiffRepository.java | 20 ++++++----
.../java/org/apache/calcite/test/QuidemTest.java | 44 ++++++++++++++--------
3 files changed, 47 insertions(+), 27 deletions(-)
diff --git a/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java b/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java
index f61185dbe4..a1408f089a 100644
--- a/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java
+++ b/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java
@@ -177,9 +177,13 @@ import static org.junit.jupiter.api.Assertions.fail;
* for details on the schema.
*
* <li>Run the test. It should fail. Inspect the output in
- * {@code build/resources/.../RelOptRulesTest_actual.xml}
- * (e.g., {@code diff src/test/resources/.../RelOptRulesTest.xml
- * build/resources/.../RelOptRulesTest_actual.xml}).
+ * {@code build/diffrepo/test/org/apache/calcite/test/RelOptRulesTest_actual.xml},
+ * e.g.
+ *
+ * <blockquote>{@code
+ * diff core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml
+ * build/diffrepo/test/org/apache/calcite/test/RelOptRulesTest_actual.xml
+ * }</blockquote>
*
* <li>Verify that the "planBefore" is the correct translation of your SQL,
* and that it contains the pattern on which your rule is supposed to fire.
diff --git a/testkit/src/main/java/org/apache/calcite/test/DiffRepository.java b/testkit/src/main/java/org/apache/calcite/test/DiffRepository.java
index f1052ea804..160d42dd8c 100644
--- a/testkit/src/main/java/org/apache/calcite/test/DiffRepository.java
+++ b/testkit/src/main/java/org/apache/calcite/test/DiffRepository.java
@@ -76,11 +76,11 @@ import static java.util.Objects.requireNonNull;
* return DiffRepository.lookup(MyTest.class);
* }
*
- * @Test public void testToUpper() {
+ * @Test void testToUpper() {
* getDiffRepos().assertEquals("${result}", "${string}");
* }
*
- * @Test public void testToLower() {
+ * @Test void testToLower() {
* getDiffRepos().assertEquals("Multi-line\nstring", "${string}");
* }
* }
@@ -110,16 +110,18 @@ import static java.util.Objects.requireNonNull;
* </code></pre></blockquote>
*
* <p>If any of the test cases fails, a log file is generated, called
- * <code>build/resources/test/com/acme/test/MyTest_actual.xml</code>, containing the actual
- * output.
+ * {@code build/diffrepo/test/com/acme/test/MyTest_actual.xml},
+ * containing the actual output.
*
* <p>The log
* file is otherwise identical to the reference log, so once the log file has
* been verified, it can simply be copied over to become the new reference
* log:
*
- * <blockquote><code>cp build/resources/test/com/acme/test/MyTest_actual.xml
- * src/test/resources/com/acme/test/MyTest.xml</code></blockquote>
+ * <blockquote>{@code
+ * cp build/diffrepo/test/com/acme/test/MyTest_actual.xml
+ * src/test/resources/com/acme/test/MyTest.xml
+ * }</blockquote>
*
* <p>If a resource or test case does not exist, <code>DiffRepository</code>
* creates them in the log file. Because DiffRepository is so forgiving, it is
@@ -128,7 +130,7 @@ import static java.util.Objects.requireNonNull;
* <p>The {@link #lookup} method ensures that all test cases share the same
* instance of the repository. This is important more than one test case fails.
* The shared instance ensures that the generated
- * <code>build/resources/test/com/acme/test/MyTest_actual.xml</code>
+ * {@code build/diffrepo/test/com/acme/test/MyTest_actual.xml}
* file contains the actual for <em>both</em> test cases.
*/
public class DiffRepository {
@@ -927,7 +929,9 @@ public class DiffRepository {
DiffRepository toRepo() {
final URL refFile = findFile(clazz, ".xml");
final String refFilePath = Sources.of(refFile).file().getAbsolutePath();
- final String logFilePath = refFilePath.replace(".xml", "_actual.xml");
+ final String logFilePath = refFilePath
+ .replace("resources", "diffrepo")
+ .replace(".xml", "_actual.xml");
final File logFile = new File(logFilePath);
assert !refFilePath.equals(logFile.getAbsolutePath());
return new DiffRepository(refFile, logFile, baseRepository, filter,
diff --git a/testkit/src/main/java/org/apache/calcite/test/QuidemTest.java b/testkit/src/main/java/org/apache/calcite/test/QuidemTest.java
index 6a8f01abde..60bcd15c9d 100644
--- a/testkit/src/main/java/org/apache/calcite/test/QuidemTest.java
+++ b/testkit/src/main/java/org/apache/calcite/test/QuidemTest.java
@@ -39,6 +39,7 @@ import com.google.common.io.PatternFilenameFilter;
import net.hydromatic.quidem.CommandHandler;
import net.hydromatic.quidem.Quidem;
+import org.checkerframework.checker.nullness.qual.Nullable;
import org.junit.jupiter.api.TestInstance;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.MethodSource;
@@ -61,6 +62,8 @@ import java.util.regex.Pattern;
import static org.junit.jupiter.api.Assertions.fail;
+import static java.util.Objects.requireNonNull;
+
/**
* Test that runs every Quidem file as a test.
*/
@@ -69,7 +72,7 @@ public abstract class QuidemTest {
private static final Pattern PATTERN = Pattern.compile("\\.iq$");
- private static Object getEnv(String varName) {
+ private static @Nullable Object getEnv(String varName) {
switch (varName) {
case "jdk18":
return System.getProperty("java.version").startsWith("1.8");
@@ -99,7 +102,7 @@ public abstract class QuidemTest {
}
}
- private Method findMethod(String path) {
+ private @Nullable Method findMethod(String path) {
// E.g. path "sql/agg.iq" gives method "testSqlAgg"
final String path1 = path.replace(File.separatorChar, '_');
final String path2 = PATTERN.matcher(path1).replaceAll("");
@@ -113,11 +116,11 @@ public abstract class QuidemTest {
return m;
}
- @SuppressWarnings("BetaApi")
+ @SuppressWarnings({"BetaApi", "UnstableApiUsage"})
protected static Collection<String> data(String first) {
// inUrl = "file:/home/fred/calcite/core/target/test-classes/sql/agg.iq"
final URL inUrl = QuidemTest.class.getResource("/" + n2u(first));
- final File firstFile = Sources.of(inUrl).file();
+ final File firstFile = Sources.of(requireNonNull(inUrl, "inUrl")).file();
final int commonPrefixLength = firstFile.getAbsolutePath().length() - first.length();
final File dir = firstFile.getParentFile();
final List<String> paths = new ArrayList<>();
@@ -137,11 +140,14 @@ public abstract class QuidemTest {
inFile = f;
outFile = new File(path + ".out");
} else {
- // e.g. path = "sql/outer.iq"
- // inUrl = "file:/home/fred/calcite/core/target/test-classes/sql/outer.iq"
+ // e.g. path = "sql/agg.iq"
+ // inUrl = "file:/home/fred/calcite/core/build/resources/test/sql/agg.iq"
+ // inFile = "/home/fred/calcite/core/build/resources/test/sql/agg.iq"
+ // outDir = "/home/fred/calcite/core/build/quidem/test/sql"
+ // outFile = "/home/fred/calcite/core/build/quidem/test/sql/agg.iq"
final URL inUrl = QuidemTest.class.getResource("/" + n2u(path));
- inFile = Sources.of(inUrl).file();
- outFile = new File(inFile.getAbsoluteFile().getParent(), u2n("surefire/") + path);
+ inFile = Sources.of(requireNonNull(inUrl, "inUrl")).file();
+ outFile = replaceDir(inFile, "resources", "quidem");
}
Util.discard(outFile.getParentFile().mkdirs());
try (Reader reader = Util.reader(inFile);
@@ -179,6 +185,18 @@ public abstract class QuidemTest {
}
}
+ /** Returns a file, replacing one directory with another.
+ *
+ * <p>For example, {@code replaceDir("/abc/str/astro.txt", "str", "xyz")}
+ * returns "{@code "/abc/xyz/astro.txt}".
+ * Note that the file name "astro.txt" does not become "axyzo.txt".
+ */
+ private static File replaceDir(File file, String target, String replacement) {
+ return new File(
+ file.getAbsolutePath().replace(n2u('/' + target + '/'),
+ n2u('/' + replacement + '/')));
+ }
+
/** Creates a command handler. */
protected CommandHandler createCommandHandler() {
return Quidem.EMPTY_COMMAND_HANDLER;
@@ -189,14 +207,8 @@ public abstract class QuidemTest {
return new QuidemConnectionFactory();
}
- /** Converts a path from Unix to native. On Windows, converts
- * forward-slashes to back-slashes; on Linux, does nothing. */
- private static String u2n(String s) {
- return File.separatorChar == '\\'
- ? s.replace('/', '\\')
- : s;
- }
-
+ /** Converts a path from native to Unix. On Windows, converts
+ * back-slashes to forward-slashes; on Linux, does nothing. */
private static String n2u(String s) {
return File.separatorChar == '\\'
? s.replace('\\', '/')