For ExprFunction, replace Environment with EvaluationSession (#559)

Also make several other classes containing implementation details internal too
partiql · Mar 28, 2022 · 18c06a8 · 18c06a8
1 parent d6ad8d2
commit 18c06a8
Show file tree

Hide file tree

Showing 31 changed files with 100 additions and 113 deletions.
diff --git a/cli/src/org/partiql/cli/functions/ReadFile.kt b/cli/src/org/partiql/cli/functions/ReadFile.kt
@@ -16,7 +16,7 @@ package org.partiql.cli.functions
 
 import com.amazon.ion.IonStruct
 import org.apache.commons.csv.CSVFormat
-import org.partiql.lang.eval.Environment
+import org.partiql.lang.eval.EvaluationSession
 import org.partiql.lang.eval.ExprValue
 import org.partiql.lang.eval.ExprValueFactory
 import org.partiql.lang.eval.io.DelimitedValues
@@ -84,7 +84,7 @@ internal class ReadFile(valueFactory: ExprValueFactory) : BaseFunction(valueFact
         "customized" to fileReadHandler(CSVFormat.DEFAULT)
     )
 
-    override fun callWithRequired(env: Environment, required: List<ExprValue>): ExprValue {
+    override fun callWithRequired(session: EvaluationSession, required: List<ExprValue>): ExprValue {
         val fileName = required[0].stringValue()
         val fileType = "ion"
         val handler = readHandlers[fileType] ?: throw IllegalArgumentException("Unknown file type: $fileType")
@@ -97,7 +97,7 @@ internal class ReadFile(valueFactory: ExprValueFactory) : BaseFunction(valueFact
         return valueFactory.newBag(seq)
     }
 
-    override fun callWithOptional(env: Environment, required: List<ExprValue>, opt: ExprValue): ExprValue {
+    override fun callWithOptional(session: EvaluationSession, required: List<ExprValue>, opt: ExprValue): ExprValue {
         val options = opt.ionValue.asIonStruct()
         val fileName = required[0].stringValue()
         val fileType = options["type"]?.stringValue() ?: "ion"

diff --git a/cli/src/org/partiql/cli/functions/WriteFile.kt b/cli/src/org/partiql/cli/functions/WriteFile.kt
@@ -16,7 +16,7 @@ package org.partiql.cli.functions
 
 import com.amazon.ion.IonStruct
 import com.amazon.ion.system.IonTextWriterBuilder
-import org.partiql.lang.eval.Environment
+import org.partiql.lang.eval.EvaluationSession
 import org.partiql.lang.eval.ExprValue
 import org.partiql.lang.eval.ExprValueFactory
 import org.partiql.lang.eval.io.DelimitedValues
@@ -63,7 +63,7 @@ internal class WriteFile(valueFactory: ExprValueFactory) : BaseFunction(valueFac
         "ion" to PRETTY_ION_WRITER
     )
 
-    override fun callWithRequired(env: Environment, required: List<ExprValue>): ExprValue {
+    override fun callWithRequired(session: EvaluationSession, required: List<ExprValue>): ExprValue {
         val fileName = required[0].stringValue()
         val fileType = "ion"
         val results = required[1]
@@ -79,7 +79,7 @@ internal class WriteFile(valueFactory: ExprValueFactory) : BaseFunction(valueFac
         }
     }
 
-    override fun callWithOptional(env: Environment, required: List<ExprValue>, opt: ExprValue): ExprValue {
+    override fun callWithOptional(session: EvaluationSession, required: List<ExprValue>, opt: ExprValue): ExprValue {
         val options = opt.ionValue.asIonStruct()
         val fileName = required[0].stringValue()
         val fileType = options["type"]?.stringValue() ?: "ion"

diff --git a/cli/test/org/partiql/cli/functions/ReadFileTest.kt b/cli/test/org/partiql/cli/functions/ReadFileTest.kt
@@ -22,8 +22,6 @@ import junit.framework.Assert.assertSame
 import org.junit.AfterClass
 import org.junit.BeforeClass
 import org.junit.Test
-import org.partiql.lang.eval.Bindings
-import org.partiql.lang.eval.Environment
 import org.partiql.lang.eval.EvaluationSession
 import org.partiql.lang.eval.ExprValue
 import org.partiql.lang.eval.ExprValueFactory
@@ -35,10 +33,7 @@ class ReadFileTest {
     private val ion = IonSystemBuilder.standard().build()
     private val valueFactory = ExprValueFactory.standard(ion)
     private val function = ReadFile(valueFactory)
-    private val env = Environment(
-        locals = Bindings.empty(),
-        session = EvaluationSession.standard()
-    )
+    private val session = EvaluationSession.standard()
 
     private fun String.exprValue() = valueFactory.newFromIonValue(ion.singleValue(this))
     private fun writeFile(path: String, content: String) = File(dirPath(path)).writeText(content)
@@ -97,7 +92,7 @@ class ReadFileTest {
         writeFile("data.ion", "1 2")
 
         val args = listOf("\"${dirPath("data.ion")}\"").map { it.exprValue() }
-        val actual = function.callWithRequired(env, args)
+        val actual = function.callWithRequired(session, args)
         val expected = "[1, 2]"
 
         assertValues(expected, actual)
@@ -109,7 +104,7 @@ class ReadFileTest {
 
         val args = listOf("\"${dirPath("data.ion")}\"").map { it.exprValue() }
         val additionalOptions = "{type:\"ion\"}".exprValue()
-        val actual = function.callWithOptional(env, args, additionalOptions)
+        val actual = function.callWithOptional(session, args, additionalOptions)
         val expected = "[1, 2]"
 
         assertValues(expected, actual)
@@ -121,7 +116,7 @@ class ReadFileTest {
 
         val args = listOf("\"${dirPath("data.csv")}\"").map { it.exprValue() }
         val additionalOptions = "{type:\"csv\"}".exprValue()
-        val actual = function.callWithOptional(env, args, additionalOptions)
+        val actual = function.callWithOptional(session, args, additionalOptions)
         val expected = "[{_1:\"1\",_2:\"2\"}]"
 
         assertValues(expected, actual)
@@ -133,7 +128,7 @@ class ReadFileTest {
 
         val args = listOf("\"${dirPath("data_with_ion_symbol_as_input.csv")}\"").map { it.exprValue() }
         val additionalOptions = "{type:csv}".exprValue()
-        val actual = function.callWithOptional(env, args, additionalOptions)
+        val actual = function.callWithOptional(session, args, additionalOptions)
         val expected = "[{_1:\"1\",_2:\"2\"}]"
 
         assertValues(expected, actual)
@@ -145,7 +140,7 @@ class ReadFileTest {
 
         val args = listOf("\"${dirPath("data_with_double_quotes_escape.csv")}\"").map { it.exprValue() }
         val additionalOptions = "{type:\"csv\"}".exprValue()
-        val actual = function.callWithOptional(env, args, additionalOptions)
+        val actual = function.callWithOptional(session, args, additionalOptions)
         val expected = "[{_1:\"1,2\",_2:\"2\"}]"
 
         assertValues(expected, actual)
@@ -157,7 +152,7 @@ class ReadFileTest {
 
         val args = listOf("\"${dirPath("data_with_double_quotes_escape.csv")}\"").map { it.exprValue() }
         val additionalOptions = "{type:\"csv\"}".exprValue()
-        val actual = function.callWithOptional(env, args, additionalOptions)
+        val actual = function.callWithOptional(session, args, additionalOptions)
         val expected = "[{_1:\"1\",_2:\"2\"},{_1:\"3\"}]"
 
         assertValues(expected, actual)
@@ -169,7 +164,7 @@ class ReadFileTest {
 
         val args = listOf("\"${dirPath("data_with_header_line.csv")}\"").map { it.exprValue() }
         val additionalOptions = "{type:\"csv\", header:true}".exprValue()
-        val actual = function.callWithOptional(env, args, additionalOptions)
+        val actual = function.callWithOptional(session, args, additionalOptions)
         val expected = "[{col1:\"1\",col2:\"2\"}]"
 
         assertValues(expected, actual)
@@ -181,7 +176,7 @@ class ReadFileTest {
 
         val args = listOf("\"${dirPath("data.tsv")}\"").map { it.exprValue() }
         val additionalOptions = "{type:\"tsv\"}".exprValue()
-        val actual = function.callWithOptional(env, args, additionalOptions)
+        val actual = function.callWithOptional(session, args, additionalOptions)
         val expected = "[{_1:\"1\",_2:\"2\"}]"
 
         assertValues(expected, actual)
@@ -193,7 +188,7 @@ class ReadFileTest {
 
         val args = listOf("\"${dirPath("data_with_header_line.tsv")}\"").map { it.exprValue() }
         val additionalOptions = "{type:\"tsv\", header:true}".exprValue()
-        val actual = function.callWithOptional(env, args, additionalOptions)
+        val actual = function.callWithOptional(session, args, additionalOptions)
         val expected = "[{col1:\"1\",col2:\"2\"}]"
 
         assertValues(expected, actual)
@@ -205,7 +200,7 @@ class ReadFileTest {
 
         val args = listOf("\"${dirPath("simple_excel.csv")}\"").map { it.exprValue() }
         val additionalOptions = "{type:\"excel_csv\", header:true}".exprValue()
-        val actual = function.callWithOptional(env, args, additionalOptions)
+        val actual = function.callWithOptional(session, args, additionalOptions)
         val expected = "[{title:\"harry potter\",category:\"book\",price:\"7.99\"}]"
 
         assertValues(expected, actual)
@@ -217,7 +212,7 @@ class ReadFileTest {
 
         val args = listOf("\"${dirPath("simple_postgresql.csv")}\"").map { it.exprValue() }
         val additionalOptions = "{type:\"postgresql_csv\", header:true}".exprValue()
-        val actual = function.callWithOptional(env, args, additionalOptions)
+        val actual = function.callWithOptional(session, args, additionalOptions)
         val expected = "[{id:\"1\",name:\"B\\\"ob\",balance:\"10000.00\"}]"
 
         assertValues(expected, actual)
@@ -229,7 +224,7 @@ class ReadFileTest {
 
         val args = listOf("\"${dirPath("simple_postgresql.txt")}\"").map { it.exprValue() }
         val additionalOptions = "{type:\"postgresql_text\", header:true}".exprValue()
-        val actual = function.callWithOptional(env, args, additionalOptions)
+        val actual = function.callWithOptional(session, args, additionalOptions)
         val expected = "[{id:\"1\",name:\"Bob\",balance:\"10000.00\"}]"
 
         assertValues(expected, actual)
@@ -241,7 +236,7 @@ class ReadFileTest {
 
         val args = listOf("\"${dirPath("simple_mysql.csv")}\"").map { it.exprValue() }
         val additionalOptions = "{type:\"mysql_csv\", header:true}".exprValue()
-        val actual = function.callWithOptional(env, args, additionalOptions)
+        val actual = function.callWithOptional(session, args, additionalOptions)
         val expected = "[{id:\"1\",name:\"B\\\"ob\",balance:\"10000.00\"}]"
 
         assertValues(expected, actual)
@@ -253,7 +248,7 @@ class ReadFileTest {
 
         val args = listOf("\"${dirPath("customized.csv")}\"").map { it.exprValue() }
         val additionalOptions = "{type:\"customized\", header:true, delimiter:' '}".exprValue()
-        val actual = function.callWithOptional(env, args, additionalOptions)
+        val actual = function.callWithOptional(session, args, additionalOptions)
         val expected = "[{id:\"1\",name:\"Bob\",balance:\"10000.00\"}]"
 
         assertValues(expected, actual)
@@ -265,7 +260,7 @@ class ReadFileTest {
 
         val args = listOf("\"${dirPath("customized.csv")}\"").map { it.exprValue() }
         val additionalOptions = "{type:\"customized\", header:true, ignore_empty_line: false}".exprValue()
-        val actual = function.callWithOptional(env, args, additionalOptions)
+        val actual = function.callWithOptional(session, args, additionalOptions)
         val expected = "[{id:\"\"},{id:\"1\",name:\"Bob\",balance:\"10000.00\"}]"
 
         assertValues(expected, actual)
@@ -277,7 +272,7 @@ class ReadFileTest {
 
         val args = listOf("\"${dirPath("customized.csv")}\"").map { it.exprValue() }
         val additionalOptions = "{type:\"customized\", header:true, ignore_surrounding_space:false, trim:false}".exprValue()
-        val actual = function.callWithOptional(env, args, additionalOptions)
+        val actual = function.callWithOptional(session, args, additionalOptions)
         val expected = "[{id:\" 1 \",name:\" Bob \",balance:\" 10000.00 \"}]"
 
         assertValues(expected, actual)
@@ -289,7 +284,7 @@ class ReadFileTest {
 
         val args = listOf("\"${dirPath("customized.csv")}\"").map { it.exprValue() }
         val additionalOptions = "{type:\"customized\", header:true, line_breaker:'\\\r\\\n'}".exprValue()
-        val actual = function.callWithOptional(env, args, additionalOptions)
+        val actual = function.callWithOptional(session, args, additionalOptions)
         val expected = "[{id:\"1\",name:\"Bob\",balance:\"10000.00\"}]"
 
         assertValues(expected, actual)
@@ -301,7 +296,7 @@ class ReadFileTest {
 
         val args = listOf("\"${dirPath("customized.csv")}\"").map { it.exprValue() }
         val additionalOptions = "{type:\"customized\", header:true, escape:'/'}".exprValue()
-        val actual = function.callWithOptional(env, args, additionalOptions)
+        val actual = function.callWithOptional(session, args, additionalOptions)
         val expected = "[{id:\"\\\"1\",name:\"Bob\",balance:\"10000.00\"}]"
 
         assertValues(expected, actual)
@@ -313,7 +308,7 @@ class ReadFileTest {
 
         val args = listOf("\"${dirPath("customized.csv")}\"").map { it.exprValue() }
         val additionalOptions = "{type:\"customized\", header:true, quote:\"'\"}".exprValue()
-        val actual = function.callWithOptional(env, args, additionalOptions)
+        val actual = function.callWithOptional(session, args, additionalOptions)
         val expected = "[{id:\"1,\",name:\"Bob\",balance:\"10000.00\"}]"
 
         assertValues(expected, actual)

diff --git a/cli/test/org/partiql/cli/functions/WriteFileTest.kt b/cli/test/org/partiql/cli/functions/WriteFileTest.kt
@@ -19,8 +19,6 @@ import org.junit.After
 import org.junit.Assert
 import org.junit.Before
 import org.junit.Test
-import org.partiql.lang.eval.Bindings
-import org.partiql.lang.eval.Environment
 import org.partiql.lang.eval.EvaluationSession
 import org.partiql.lang.eval.ExprValueFactory
 import java.io.File
@@ -29,10 +27,7 @@ class WriteFileTest {
     private val ion = IonSystemBuilder.standard().build()
     private val valueFactory = ExprValueFactory.standard(ion)
     private val function = WriteFile(valueFactory)
-    private val env = Environment(
-        locals = Bindings.empty(),
-        session = EvaluationSession.standard()
-    )
+    private val session = EvaluationSession.standard()
 
     private fun String.exprValue() = valueFactory.newFromIonValue(ion.singleValue(this))
     private fun readFile(path: String) = File(dirPath(path)).readText()
@@ -52,7 +47,7 @@ class WriteFileTest {
     @Test
     fun writeIonAsDefault() {
         val args = listOf(""" "${dirPath("data.ion")}" """, "[1, 2]").map { it.exprValue() }
-        function.callWithRequired(env, args).ionValue
+        function.callWithRequired(session, args).ionValue
 
         val expected = "1 2"
 
@@ -63,7 +58,7 @@ class WriteFileTest {
     fun readIon() {
         val args = listOf(""" "${dirPath("data1.ion")}" """, "[1, 2]").map { it.exprValue() }
         val additionalOptions = """{type: "ion"}""".exprValue()
-        function.callWithOptional(env, args, additionalOptions).ionValue
+        function.callWithOptional(session, args, additionalOptions).ionValue
 
         val expected = "1 2"
 

diff --git a/examples/src/kotlin/org/partiql/examples/CustomFunctionsExample.kt b/examples/src/kotlin/org/partiql/examples/CustomFunctionsExample.kt
@@ -4,7 +4,6 @@ import com.amazon.ion.system.IonSystemBuilder
 import org.partiql.examples.util.Example
 import org.partiql.lang.CompilerPipeline
 import org.partiql.lang.errors.ErrorCode
-import org.partiql.lang.eval.Environment
 import org.partiql.lang.eval.EvaluationException
 import org.partiql.lang.eval.EvaluationSession
 import org.partiql.lang.eval.ExprFunction
@@ -42,7 +41,7 @@ class FibScalarExprFunc(private val valueFactory: ExprValueFactory) : ExprFuncti
         returnType = StaticType.INT
     )
 
-    override fun callWithRequired(env: Environment, required: List<ExprValue>): ExprValue {
+    override fun callWithRequired(session: EvaluationSession, required: List<ExprValue>): ExprValue {
         // [NullPropagatingExprFunction] also checks arity of the function call, so
         // there is no need to ensure [args] is the correct size.
         // However, at the moment there is no facility for ensuring that the arguments are
@@ -76,7 +75,7 @@ class FibListExprFunc(private val valueFactory: ExprValueFactory) : ExprFunction
         returnType = StaticType.LIST
     )
 
-    override fun callWithRequired(env: Environment, required: List<ExprValue>): ExprValue {
+    override fun callWithRequired(session: EvaluationSession, required: List<ExprValue>): ExprValue {
         val argN = required.first()
         if (argN.type != ExprValueType.INT) {
             throw EvaluationException("Argument to fib_list was not an integer", ErrorCode.INTERNAL_ERROR, internal = false)

diff --git a/gradle.properties b/gradle.properties
@@ -1,2 +1,2 @@
 org.gradle.parallel=true
-org.gradle.caching=true
+org.gradle.caching=true
diff --git a/lang/src/org/partiql/lang/eval/Environment.kt b/lang/src/org/partiql/lang/eval/Environment.kt
@@ -25,7 +25,7 @@ import java.util.TreeMap
  * @param session The evaluation session.
  * @param groups The map of [Group]s that is currently being built during query execution.
  */
-data class Environment(
+internal data class Environment(
     internal val locals: Bindings<ExprValue>,
     val current: Bindings<ExprValue> = locals,
     val session: EvaluationSession,

diff --git a/lang/src/org/partiql/lang/eval/EvaluatingCompiler.kt b/lang/src/org/partiql/lang/eval/EvaluatingCompiler.kt
@@ -1011,12 +1011,12 @@ internal class EvaluatingCompiler(
         val computeThunk = when (func.signature.unknownArguments) {
             UnknownArguments.PROPAGATE -> thunkFactory.thunkEnvOperands(metas, funcArgThunks) { env, values ->
                 val checkedArgs = checkArgumentTypes(func.signature, values)
-                func.call(env, checkedArgs)
+                func.call(env.session, checkedArgs)
             }
             UnknownArguments.PASS_THRU -> thunkFactory.thunkEnv(metas) { env ->
                 val funcArgValues = funcArgThunks.map { it(env) }
                 val checkedArgs = checkArgumentTypes(func.signature, funcArgValues)
-                func.call(env, checkedArgs)
+                func.call(env.session, checkedArgs)
             }
         }
 

diff --git a/lang/src/org/partiql/lang/eval/ExprAggregator.kt b/lang/src/org/partiql/lang/eval/ExprAggregator.kt
@@ -23,7 +23,7 @@ package org.partiql.lang.eval
  * operand.  The evaluator's responsibility is to effectively compile this definition
  * into a form of [ExprFunction] that operates over the collection as an [ExprValue].
  */
-interface ExprAggregator {
+internal interface ExprAggregator {
     /** Accumulates the next value into this [ExprAggregator]. */
     fun next(value: ExprValue)
 

diff --git a/lang/src/org/partiql/lang/eval/ExprAggregatorFactory.kt b/lang/src/org/partiql/lang/eval/ExprAggregatorFactory.kt
@@ -19,7 +19,7 @@ package org.partiql.lang.eval
  *
  * This is the entry point for aggregate function definitions in the evaluator.
  */
-interface ExprAggregatorFactory {
+internal interface ExprAggregatorFactory {
     companion object {
         fun over(func: () -> ExprAggregator): ExprAggregatorFactory =
             object : ExprAggregatorFactory {