Undefined Variable Behavior

[johnedquinn]

Overview

I have a PR (partiql/partiql-tests#124) to fix several conformance tests that deal with a common theme: undefined variables. Before this PR, identifiers that were not resolved to environment bindings or to a database name would return MISSING -- it would not fail compilation. This PR fixes this by failing compilation in these scenarios.

The PartiQL Team has conflicting views on this topic, and I hope that this discussion may bring forth an actionable conclusion.

Specification

From my reading of the PartiQL Specification, a database type environment exists and always "knows" which "names" are present (regardless of whether the value itself is schema-less). From the PartiQL Specification:

Similarly, for a given 𝑞 at compile (i.e. planning) time, a database type environment... and variables type environment ... are defined. The type environment is a binding tuple ... where each 𝑥𝑖 is a name that is unique and binds to the PartiQL type𝜏𝑖. For schema-less values, 𝜏 can be considered the union of any possible type (for which all operations are potentially applicable).

-- PartiQL Specification Section 3.2

Therefore, even in a schema-less database, a binding's name must be present in the database type environment.

In Section 10.1, the specification elaborates on references to variables and database names:

2. in a FROM clause path that starts with identifier, the identifier refers to the database name identifier; if there is no such database name, the refers to a variable; otherwise query fails compilation. [5]
3. in a non-FROM clause path that starts with identifier, the identifier refers to the environment variable named identifier; if there is no such environment variable, the identifier refers to the database name identifier; if there is no such database name either, the query fails compilation.

The PartiQL Specification states that the "query fails compilation" if an identifier cannot be resolved to either an environment variable or a database name.

Other Relevant Issues

PLK, for reasons unbeknownst to me, has added an API called UndefinedVariableBehavior. The conformance tests use the MISSING variant of this enum.

Question

1. Is the PR (Fixes undefined variable behavior tests partiql-tests#124) correct in making these tests fail compilation? Or, should they return MISSING?
2. In PLK 1.0 (the future stable version of PLK), should we add an equivalent UndefinedVariableBehavior for compatibility purposes?

[jpschorr]

Last time I looked into UndefinedVariableBehavior, I came to believe it was added to solve a different problem (local 'dynamic lookup` in the face of totally dynamic, unresolved plans. The fact that it has this behavior w.r.t. the global environment wasn't, I believe, it's original intent.

[almann]

My opinion here is that undefined variables should be static errors. It is a misfeature in our implementations that we have any kind of dynamic scoping (and I am probably to blame for some of that in Kotlin). It was a relic when PartiQL had more loose dynamic scoping rules that we quickly got rid of for query stability.

[am357]

My opinion here is that undefined variables should be static errors. It is a misfeature in our implementations that we have any kind of dynamic scoping (and I am probably to blame for some of that in Kotlin). It was a relic when PartiQL had more loose dynamic scoping rules that we quickly got rid of for query stability.

Makes sense @almann. Do you mind sharing your thoughts on the behavior for backward compatibility for Kotlin implementation. From your point of view, should the 1.0 break this behavior or we keep the behavior not necessarily through the current flag [UndefinedVariableBehavior](https://github.com/partiql/partiql-lang-kotlin/blob/306ec7c1dc143660d6d861cc9626cbc3ac444ec6/partiql-lang/src/main/kotlin/org/partiql/lang/eval/CompileOptions.kt#L34 and through some other means.