Merges main into V1

.editorconfig

@@ -0,0 +1,3 @@
+# Ignore custom ktlint rules for tests
+[**/test/**.kt]
+disabled_rules = custom-ktlint-rules:top-level-internal,custom-ktlint-rules:top-level-public

.github/workflows/build.yml

@@ -40,6 +40,11 @@ jobs:
           GRADLE_OPTS: -Dorg.gradle.jvmargs="-XX:MaxMetaspaceSize=1g"
         run: ./gradlew build jacocoTestReport
 
+      # Custom ktlint rules are only run when the `custom-ktlint-rules` property is set.
+      # Once these rules are run by default, this GH Action step can be removed.
+      - name: Run custom ktlint rules
+        run: ./gradlew ktlintCheck -Pcustom-ktlint-rules
+
       # Upload coverage for CLI, LANG, PTS, TEST_SCRIPT, and EXAMPLES
       - name: Upload CLI coverage
         uses: codecov/codecov-action@v3

CHANGELOG.md

@@ -21,8 +21,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Contributors
 Thank you to all who have contributed!
-- @<your-username>
-
 -->
 
 ## [Unreleased]
@@ -43,6 +41,38 @@ Thank you to all who have contributed!
 Thank you to all who have contributed!
 - @<your-username>
 
+## [0.14.5]
+
+### Added
+- partiql-ast: adds warning not to implement `AstVisitor` interface directly. Please extend `AstBaseVisitor` instead.
+- partiql-plan: adds warning not to implement `PlanVisitor` interface directly. Please extend `PlanBaseVisitor` instead.
+
+### Changed
+- Change `StaticType.AnyOfType`'s `.toString` to not perform `.flatten()`
+- Change modeling of `COALESCE` and `NULLIF` to dedicated nodes in logical plan
+- Function resolution logic: Now the function resolver would match all possible candidate (based on if the argument can be coerced to the Signature parameter type). If there are multiple match it will first attempt to pick the one requires the least cast, then pick the function with the highest precedence.
+- **Behavioral change**: The COUNT aggregate function now returns INT64.
+
+### Deprecated
+- The current SqlBlock, SqlDialect, and SqlLayout are marked as deprecated and will be slightly changed in the next release. 
+- Deprecates constructor and properties `variableName` and `caseSensitive` of `org.partiql.planner.PlanningProblemDetails.UndefinedVariable`
+  in favor of newly added constructor and properties `name` and `inScopeVariables`.
+
+### Fixed
+- `StaticType.flatten()` on an `AnyOfType` with `AnyType` will return `AnyType`
+- Updates the default `.sql()` method to use a more efficient (internal) printer implementation.
+- Fixes aggregations of attribute references to values of union types. This fix also allows for proper error handling by passing the UnknownAggregateFunction problem to the ProblemCallback. Please note that, with this change, the planner will no longer immediately throw an IllegalStateException for this exact scenario.
+
+### Removed
+
+### Security
+
+### Contributors
+Thank you to all who have contributed!
+- @rchowell
+- @alancai98
+- @johnedquinn
+
 ## [1.0.0-perf.1] - 2024-03-04
 
 This is a pre-release containing:

README.md

@@ -7,7 +7,7 @@
 # PartiQL Lang Kotlin
 
 This is a Kotlin implementation of the [PartiQL specification](https://partiql.org/assets/PartiQL-Specification.pdf).
-PartiQL is based on SQL-92 and has added support for working with schemaless hierarchical data.
+PartiQL is based on SQL-99 and has added support for working with schemaless hierarchical data.
 PartiQL’s extensions to SQL are easy to understand, treat nested data as first class citizens and
 compose seamlessly with each other and SQL.
 
@@ -31,7 +31,7 @@ This project is published to [Maven Central](https://search.maven.org/artifact/o
 
 | Group ID      | Artifact ID           | Recommended Version |
 |---------------|-----------------------|---------------------|
-| `org.partiql` | `partiql-lang-kotlin` | `0.14.4`            |
+| `org.partiql` | `partiql-lang-kotlin` | `0.14.5`            |
 
 
 For Maven builds, add the following to your `pom.xml`:
@@ -86,13 +86,33 @@ This will build the reference interpreter and test framework, then run all unit
 
 ## Directory Structure
 
-- `docs` documentation and migration guides as well as source for the GitHub Wiki
-- `examples`
-- `lib` libraries not part of the partiql-lang-kotlin JAR
-- `partiql-cli` contains the source code of the command-line interface and interactive prompt. (CLI/REPL)
-- `partiql-lang` source code for the PartiQL parser and interpreter.
-- `paritql-lang/src/jmh` contains the JMH benchmarks for PartiQL.
-- `partiql-types` PartiQL type system
+```
+$ tree -d -L 2 -I build -I src`
+.
+├── buildSrc                    Gradle multi-project build
+├── docs                        Documentation
+│   ├── upgrades
+│   └── wiki
+├── examples                    Code examples
+├── lib
+│   ├── isl                     Ion Schema DOM
+│   └── sprout                  IR codegen
+├── partiql-ast                 PartiQL ast data structures and utilities
+├── partiql-cli                 CLI & Shell application
+├── partiql-coverage            Code coverage library
+├── partiql-lang                Top-level project containing all subprojects
+├── partiql-parser              PartiQL parser
+├── partiql-plan                PartiQL plan data structures and utilities
+├── partiql-planner             PartiQL planner
+├── partiql-spi                 Service provider interface
+├── partiql-types               PartiQL types
+├── plugins                     PartiQL plugins used in testing 
+│   ├── partiql-local
+│   └── partiql-memory
+└── test
+    ├── partiql-tests           Conformance test data
+    └── partiql-tests-runner    Conformance test runner
+```
 
 ### Running JMH Benchmarks
 

buildSrc/build.gradle.kts

@@ -27,7 +27,7 @@ object Versions {
     const val detekt = "1.20.0-RC1"
     const val dokka = "1.6.10"
     const val kotlin = "1.6.20"
-    const val ktlint = "10.2.1"
+    const val ktlintGradle = "10.2.1"
     const val pig = "0.6.1"
 }
 
@@ -36,15 +36,15 @@ object Plugins {
     const val detekt = "io.gitlab.arturbosch.detekt:detekt-gradle-plugin:${Versions.detekt}"
     const val dokka = "org.jetbrains.dokka:dokka-gradle-plugin:${Versions.dokka}"
     const val kotlinGradle = "org.jetbrains.kotlin:kotlin-gradle-plugin:${Versions.kotlin}"
-    const val ktlint = "org.jlleitschuh.gradle:ktlint-gradle:${Versions.ktlint}"
+    const val ktlintGradle = "org.jlleitschuh.gradle:ktlint-gradle:${Versions.ktlintGradle}"
     const val pig = "org.partiql:pig-gradle-plugin:${Versions.pig}"
 }
 
 dependencies {
     implementation(Plugins.detekt)
     implementation(Plugins.dokka)
     implementation(Plugins.kotlinGradle)
-    implementation(Plugins.ktlint)
+    implementation(Plugins.ktlintGradle)
     implementation(Plugins.pig)
     implementation(Plugins.binaryCompatibilityValidator)
 }

buildSrc/src/main/kotlin/partiql.conventions.gradle.kts

@@ -40,6 +40,12 @@ dependencies {
     testImplementation(Deps.kotlinTest)
     testImplementation(Deps.kotlinTestJunit)
     testImplementation(Deps.junitParams)
+    // Custom ktlint rules are added by adding to the `dependencies` block: https://github.com/JLLeitschuh/ktlint-gradle/tree/v10.2.0?tab=readme-ov-file#configuration
+    // Currently, we only run the rules when the `custom-ktlint-rules` property is set.
+    // Once we enable the custom rules to run by default, this conditional can be removed.
+    if (hasProperty("custom-ktlint-rules")) {
+        ktlintRuleset(project(":custom-ktlint-rules"))
+    }
 }
 
 java {
@@ -73,6 +79,13 @@ tasks.compileTestKotlin {
 }
 
 configure<KtlintExtension> {
+    version.set(Versions.ktlint)
+    // Currently set `ktlintCheck` to not fail on the custom rules.
+    // Once we enable the custom rules to run by default, this conditional can be removed.
+    if (hasProperty("custom-ktlint-rules")) {
+        ignoreFailures.set(true)
+    }
+    outputToConsole.set(true)
     filter {
         exclude { it.file.path.contains(generatedSrc) }
     }

buildSrc/src/main/kotlin/partiql.versions.kt

@@ -45,10 +45,10 @@ object Versions {
     const val kotlinxCollections = "0.3.5"
     const val picoCli = "4.7.0"
     const val kasechange = "1.3.0"
-    const val ktlint = "11.6.0"
     const val pig = "0.6.2"
     const val kotlinxCoroutines = "1.6.0"
     const val kotlinxCoroutinesJdk8 = "1.6.0"
+    const val ktlint = "0.42.1" // we're on an old version of ktlint. TODO upgrade https://github.com/partiql/partiql-lang-kotlin/issues/1418
 
     // Testing
     const val assertj = "3.11.0"
@@ -92,6 +92,7 @@ object Deps {
     const val pigRuntime = "org.partiql:partiql-ir-generator-runtime:${Versions.pig}"
     const val kotlinxCoroutines = "org.jetbrains.kotlinx:kotlinx-coroutines-core:${Versions.kotlinxCoroutines}"
     const val kotlinxCoroutinesJdk8 = "org.jetbrains.kotlinx:kotlinx-coroutines-jdk8:${Versions.kotlinxCoroutinesJdk8}"
+    const val ktlint = "com.pinterest.ktlint:ktlint-core:${Versions.ktlint}"
 
     // Testing
     const val assertj = "org.assertj:assertj-core:${Versions.assertj}"
@@ -106,6 +107,7 @@ object Deps {
     const val mockito = "org.mockito:mockito-junit-jupiter:${Versions.mockito}"
     const val mockk = "io.mockk:mockk:${Versions.mockk}"
     const val kotlinxCoroutinesTest = "org.jetbrains.kotlinx:kotlinx-coroutines-test:${Versions.kotlinxCoroutinesTest}"
+    const val ktlintTest = "com.pinterest.ktlint:ktlint-test:${Versions.ktlint}"
 
     // JMH Benchmarking
     const val jmhCore = "org.openjdk.jmh:jmh-core:${Versions.jmhCore}"
@@ -125,7 +127,6 @@ object Plugins {
     const val detekt = "io.gitlab.arturbosch.detekt"
     const val dokka = "org.jetbrains.dokka"
     const val jmh = "me.champeau.gradle.jmh"
-    const val ktlint = "org.jlleitschuh.gradle.ktlint"
     const val library = "org.gradle.java-library"
     const val testFixtures = "org.gradle.java-test-fixtures"
 }

custom-ktlint-rules/build.gradle.kts

@@ -0,0 +1,16 @@
+plugins {
+    id(Plugins.conventions)
+    `java-library`
+}
+
+dependencies {
+    implementation(Deps.ktlint)
+
+    testImplementation(Deps.assertj)
+    testImplementation(Deps.ktlintTest)
+    testImplementation(Deps.junitParams)
+}
+
+repositories {
+    mavenCentral()
+}

custom-ktlint-rules/src/main/kotlin/org/partiql/ktlint/CustomRuleSetProvider.kt

@@ -0,0 +1,10 @@
+package org.partiql.ktlint
+
+import com.pinterest.ktlint.core.RuleSet
+import com.pinterest.ktlint.core.RuleSetProvider
+import org.partiql.ktlint.rule.TopLevelInternalRule
+import org.partiql.ktlint.rule.TopLevelPublicRule
+
+class CustomRuleSetProvider : RuleSetProvider {
+    override fun get(): RuleSet = RuleSet("custom-ktlint-rules", TopLevelInternalRule(), TopLevelPublicRule())
+}

custom-ktlint-rules/src/main/kotlin/org/partiql/ktlint/rule/TopLevelInternalRule.kt

@@ -0,0 +1,38 @@
+package org.partiql.ktlint.rule
+
+import com.pinterest.ktlint.core.Rule
+import com.pinterest.ktlint.core.ast.ElementType
+import com.pinterest.ktlint.core.ast.children
+import org.jetbrains.kotlin.com.intellij.lang.ASTNode
+
+public class TopLevelInternalRule : Rule("top-level-internal") {
+
+    override fun visit(
+        node: ASTNode,
+        autoCorrect: Boolean,
+        emit: (offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit
+    ) {
+        if (node.elementType != ElementType.IDENTIFIER) {
+            return
+        }
+
+        // Focus on just functions and values
+        val parent = node.treeParent
+        if (parent.elementType != ElementType.FUN && parent.elementType != ElementType.PROPERTY) {
+            return
+        }
+
+        // Check grandparent of node is FILE; if so, is top-level declaration
+        if (parent.treeParent.elementType != ElementType.FILE) {
+            return
+        }
+        val modifiers = parent.findChildByType(ElementType.MODIFIER_LIST)?.children()
+        if (modifiers != null && modifiers.any { it.elementType == ElementType.INTERNAL_KEYWORD }) {
+            emit(
+                node.startOffset,
+                "Top-level internal declaration found: ${node.text}",
+                false
+            )
+        }
+    }
+}

custom-ktlint-rules/src/main/kotlin/org/partiql/ktlint/rule/TopLevelPublicRule.kt

@@ -0,0 +1,63 @@
+package org.partiql.ktlint.rule
+
+import com.pinterest.ktlint.core.Rule
+import com.pinterest.ktlint.core.ast.ElementType
+import com.pinterest.ktlint.core.ast.children
+import org.jetbrains.kotlin.com.intellij.lang.ASTNode
+
+public class TopLevelPublicRule : Rule("top-level-public") {
+    override fun visit(
+        node: ASTNode,
+        autoCorrect: Boolean,
+        emit: (offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit
+    ) {
+        if (node.elementType != ElementType.IDENTIFIER) {
+            return
+        }
+
+        // Focus on just functions and values
+        val parent = node.treeParent
+        if (parent.elementType != ElementType.FUN && parent.elementType != ElementType.PROPERTY) {
+            return
+        }
+
+        // Check grandparent of node is FILE; if so, is top-level declaration
+        val grandParent = parent.treeParent
+        if (grandParent.elementType != ElementType.FILE) {
+            return
+        }
+
+        val modifiers = parent.findChildByType(ElementType.MODIFIER_LIST)
+        if (modifiers != null && modifiers.isNotPublic()) {
+            return
+        }
+
+        val annotationEntry = grandParent.findChildByType(ElementType.FILE_ANNOTATION_LIST)?.findChildByType(ElementType.ANNOTATION_ENTRY)
+        if (annotationEntry == null || !annotationEntry.containsFileJvmName()) {
+            emit(
+                node.startOffset,
+                "Top-level public declaration found without `@file:JvmName` annotation: ${node.text}",
+                false
+            )
+        }
+    }
+
+    // returns true iff modifiers is not one of `PRIVATE_KEYWORD`, `INTERNAL_KEYWORD` or `PROTECTED_KEYWORD`
+    private fun ASTNode.isNotPublic(): Boolean {
+        val modifiers = this.children().map { it.elementType }
+        return modifiers.any { it == ElementType.PRIVATE_KEYWORD || it == ElementType.INTERNAL_KEYWORD || it == ElementType.PROTECTED_KEYWORD }
+    }
+
+    // returns true iff node is `@file:JvmName(<some name>)`
+    private fun ASTNode.containsFileJvmName(): Boolean {
+        val annotationTarget = this.findChildByType(ElementType.ANNOTATION_TARGET)
+        if (annotationTarget == null || annotationTarget.text.lowercase() != "file") {
+            return false
+        }
+        val constructorCallee = this.findChildByType(ElementType.CONSTRUCTOR_CALLEE)
+        if (constructorCallee == null || constructorCallee.text.lowercase() != "jvmname") {
+            return false
+        }
+        return true
+    }
+}

custom-ktlint-rules/src/main/resources/META-INF/services/com.pinterest.ktlint.core.RuleSetProvider

@@ -0,0 +1 @@
+org.partiql.ktlint.CustomRuleSetProvider

custom-ktlint-rules/src/test/kotlin/org/partiql/ktlint/rule/TopLevelInternalRuleTest.kt

@@ -0,0 +1,48 @@
+package org.partiql.ktlint.rule
+
+import com.pinterest.ktlint.core.LintError
+import com.pinterest.ktlint.test.lint
+import org.assertj.core.api.Assertions
+import org.junit.jupiter.api.Test
+
+class TopLevelInternalRuleTest {
+    @Test
+    fun `top-level internal`() {
+        val code =
+            """
+            internal fun internalTopLevelFun() {}   // ktlint error
+
+            internal val internalTopLevelVal = 123  // ktlint error
+
+            internal var internalTopLevelVar = 456    // ktlint error
+
+            // No errors for below (for this rule)
+            public fun publicTopLevelFun() {}
+
+            public val publicTopLevelVal = 123
+
+            public var publicTopLevelVar = 456
+
+            fun publicTopLevelFun2() {}
+
+            val publicTopLevelVal = 123
+
+            var publicTopLevelVar = 456
+
+            public class PublicClass {
+                internal fun internalFun() {}
+
+                internal val internalVal = 123
+
+                public fun publicFun() {}
+
+                public val publicVal = 123
+            }
+            """.trimIndent()
+        Assertions.assertThat(TopLevelInternalRule().lint(code)).containsExactly(
+            LintError(1, 14, "top-level-internal", "Top-level internal declaration found: internalTopLevelFun"),
+            LintError(3, 14, "top-level-internal", "Top-level internal declaration found: internalTopLevelVal"),
+            LintError(5, 14, "top-level-internal", "Top-level internal declaration found: internalTopLevelVar")
+        )
+    }
+}