Limit Postgres MIN/MAX to an arity of one

cashapp · Aug 15, 2023 · dbc6c3f · dbc6c3f
1 parent 4b3602c
commit dbc6c3f
Show file tree

Hide file tree

Showing 5 changed files with 45 additions and 5 deletions.
diff --git a/...tgresql/src/main/kotlin/app/cash/sqldelight/dialects/postgresql/PostgreSqlTypeResolver.kt b/...tgresql/src/main/kotlin/app/cash/sqldelight/dialects/postgresql/PostgreSqlTypeResolver.kt
@@ -21,6 +21,7 @@ import app.cash.sqldelight.dialects.postgresql.grammar.psi.PostgreSqlExtensionEx
 import app.cash.sqldelight.dialects.postgresql.grammar.psi.PostgreSqlInsertStmt
 import app.cash.sqldelight.dialects.postgresql.grammar.psi.PostgreSqlTypeName
 import app.cash.sqldelight.dialects.postgresql.grammar.psi.PostgreSqlUpdateStmtLimited
+import com.alecstrong.sql.psi.core.SqlAnnotationHolder
 import com.alecstrong.sql.psi.core.psi.SqlColumnExpr
 import com.alecstrong.sql.psi.core.psi.SqlCreateTableStmt
 import com.alecstrong.sql.psi.core.psi.SqlExpr
@@ -110,6 +111,21 @@ class PostgreSqlTypeResolver(private val parentResolver: TypeResolver) : TypeRes
     else -> null
   }
 
+  override fun SqlFunctionExpr.validateFunction(
+    annotationHolder: SqlAnnotationHolder,
+  ) {
+    when (functionName.text.lowercase()) {
+      "max", "min" -> {
+        if (exprList.size != 1) {
+          annotationHolder.createErrorAnnotation(
+            this,
+            "${functionName.text} only takes one argument",
+          )
+        }
+      }
+    }
+  }
+
   override fun queryWithResults(sqlStmt: SqlStmt): QueryWithResults? {
     sqlStmt.insertStmt?.let { insert ->
       check(insert is PostgreSqlInsertStmt)

diff --git a/dialects/postgresql/src/test/fixtures_postgresql/function-arity/Test.s b/dialects/postgresql/src/test/fixtures_postgresql/function-arity/Test.s
@@ -0,0 +1,13 @@
+CREATE TABLE minMaxTable(
+  first INTEGER,
+  second INTEGER
+);
+
+SELECT MAX(first) FROM minMaxTable;
+SELECT MIN(first) FROM minMaxTable;
+
+-- error[col 7]: MAX only takes one argument
+SELECT MAX(first, second) FROM minMaxTable;
+
+-- error[col 7]: MIN only takes one argument
+SELECT MIN(first, second) FROM minMaxTable;
diff --git a/sqldelight-compiler/dialect/src/main/kotlin/app/cash/sqldelight/dialect/api/TypeResolver.kt b/sqldelight-compiler/dialect/src/main/kotlin/app/cash/sqldelight/dialect/api/TypeResolver.kt
@@ -1,5 +1,6 @@
 package app.cash.sqldelight.dialect.api
 
+import com.alecstrong.sql.psi.core.SqlAnnotationHolder
 import com.alecstrong.sql.psi.core.psi.SqlExpr
 import com.alecstrong.sql.psi.core.psi.SqlFunctionExpr
 import com.alecstrong.sql.psi.core.psi.SqlStmt
@@ -23,6 +24,8 @@ interface TypeResolver {
    */
   fun functionType(functionExpr: SqlFunctionExpr): IntermediateType?
 
+  fun SqlFunctionExpr.validateFunction(annotationHolder: SqlAnnotationHolder) {}
+
   /**
    * @return the dialect specific (or [PrimitiveType]) for a type name.
    */

diff --git a/sqldelight-compiler/src/main/kotlin/app/cash/sqldelight/core/lang/psi/FunctionExprMixin.kt b/sqldelight-compiler/src/main/kotlin/app/cash/sqldelight/core/lang/psi/FunctionExprMixin.kt
@@ -9,11 +9,16 @@ import com.intellij.lang.ASTNode
 
 internal class FunctionExprMixin(node: ASTNode?) : SqlFunctionExprImpl(node) {
   override fun annotate(annotationHolder: SqlAnnotationHolder) {
-    if (parent is SqlResultColumn &&
-      typeResolver != AnsiSqlTypeResolver &&
-      typeResolver.functionType(this) == null
-    ) {
-      annotationHolder.createErrorAnnotation(this, "Unknown function ${functionName.text}")
+    if (parent is SqlResultColumn) {
+      if (typeResolver != AnsiSqlTypeResolver &&
+        typeResolver.functionType(this) == null
+      ) {
+        annotationHolder.createErrorAnnotation(this, "Unknown function ${functionName.text}")
+      } else {
+        with(typeResolver) {
+          validateFunction(annotationHolder)
+        }
+      }
     }
   }
 }
diff --git a/sqldelight-compiler/src/test/kotlin/app/cash/sqldelight/core/queries/ExpressionTest.kt b/sqldelight-compiler/src/test/kotlin/app/cash/sqldelight/core/queries/ExpressionTest.kt
@@ -8,6 +8,7 @@ import app.cash.sqldelight.core.compiler.SelectQueryGenerator
 import app.cash.sqldelight.core.dialects.blobType
 import app.cash.sqldelight.core.dialects.textType
 import app.cash.sqldelight.test.util.FixtureCompiler
+import com.alecstrong.sql.psi.core.psi.SqlFunctionExpr
 import com.google.common.truth.Truth.assertThat
 import com.squareup.burst.BurstJUnit4
 import com.squareup.kotlinpoet.DOUBLE
@@ -404,6 +405,8 @@ class ExpressionTest {
       else -> LONG
     }
 
+    val f = file.children.filterIsInstance<SqlFunctionExpr>()
+
     val query = file.namedQueries.first()
     assertThat(query.resultColumns.map { it.javaType }).containsExactly(
       ByteArray::class.asClassName().copy(nullable = true),