Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Query planner passes #588

Merged
merged 7 commits into from
May 19, 2022
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion lang/resources/org/partiql/type-domains/partiql.ion
Original file line number Diff line number Diff line change
Expand Up @@ -725,7 +725,7 @@ may then be further optimized by selecting better implementations of each operat
join_type::join_type
left::bexpr
right::bexpr
predicate::expr)
predicate::(? expr))


(offset i::impl row_count::expr source::bexpr)
Expand Down
38 changes: 26 additions & 12 deletions lang/src/org/partiql/lang/domains/util.kt
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package org.partiql.lang.domains

import com.amazon.ionelement.api.IonElement
import com.amazon.ionelement.api.MetaContainer
import com.amazon.ionelement.api.emptyMetaContainer
import com.amazon.ionelement.api.metaContainerOf
Expand All @@ -14,6 +15,19 @@ import org.partiql.lang.eval.BindingCase
fun PartiqlAst.Builder.id(name: String) =
id(name, caseInsensitive(), unqualified())

// TODO: once https://github.com/partiql/partiql-ir-generator/issues/6 has been completed, we can delete this.
fun PartiqlLogical.Builder.id(name: String) =
id(name, caseInsensitive(), unqualified())

// TODO: once https://github.com/partiql/partiql-ir-generator/issues/6 has been completed, we can delete this.
fun PartiqlLogical.Builder.pathExpr(exp: PartiqlLogical.Expr) =
pathExpr(exp, caseInsensitive())

// Workaround for a bug in PIG that is fixed in its next release:
// https://github.com/partiql/partiql-ir-generator/issues/41
fun List<IonElement>.asAnyElement() =
this.map { it.asAnyElement() }
Comment on lines +26 to +29
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While at it, shouldn't we just release the PIG and remove this?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, but partiql/partiql-ir-generator#41 hasn't been fixed yet.


val MetaContainer.staticType: StaticTypeMeta? get() = this[StaticTypeMeta.TAG] as StaticTypeMeta?

/** Constructs a container with the specified metas. */
Expand Down Expand Up @@ -60,17 +74,17 @@ fun PartiqlAst.CaseSensitivity.toBindingCase(): BindingCase = when (this) {
}

/**
* Returns the [SourceLocationMeta] as an error context if the [SourceLocationMeta.TAG] exists in the passed
* [metaContainer]. Otherwise, returns an empty map.
* Converts a [PartiqlLogical.CaseSensitivity] to a [BindingCase].
*/
fun errorContextFrom(metaContainer: MetaContainer?): PropertyValueMap {
if (metaContainer == null) {
return PropertyValueMap()
}
val location = metaContainer[SourceLocationMeta.TAG] as? SourceLocationMeta
return if (location != null) {
org.partiql.lang.eval.errorContextFrom(location)
} else {
PropertyValueMap()
}
fun PartiqlLogical.CaseSensitivity.toBindingCase(): BindingCase = when (this) {
is PartiqlLogical.CaseSensitivity.CaseInsensitive -> BindingCase.INSENSITIVE
is PartiqlLogical.CaseSensitivity.CaseSensitive -> BindingCase.SENSITIVE
}

/**
* Converts a [PartiqlLogical.CaseSensitivity] to a [BindingCase].
*/
fun PartiqlPhysical.CaseSensitivity.toBindingCase(): BindingCase = when (this) {
is PartiqlPhysical.CaseSensitivity.CaseInsensitive -> BindingCase.INSENSITIVE
is PartiqlPhysical.CaseSensitivity.CaseSensitive -> BindingCase.SENSITIVE
}
2 changes: 1 addition & 1 deletion lang/src/org/partiql/lang/eval/EvaluatingCompiler.kt
Original file line number Diff line number Diff line change
Expand Up @@ -3058,7 +3058,7 @@ private class SingleProjectionElement(val name: ExprValue, val thunk: ThunkEnv)
*/
private class MultipleProjectionElement(val thunks: List<ThunkEnv>) : ProjectionElement()

private val MetaContainer.sourceLocationMeta get() = this[SourceLocationMeta.TAG] as? SourceLocationMeta
internal val MetaContainer.sourceLocationMeta get() = this[SourceLocationMeta.TAG] as? SourceLocationMeta

private fun StaticType.getTypes() = when (val flattened = this.flatten()) {
is AnyOfType -> flattened.types
Expand Down
2 changes: 2 additions & 0 deletions lang/src/org/partiql/lang/eval/builtins/BuiltinFunctions.kt
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ import org.partiql.lang.types.FunctionSignature
import org.partiql.lang.types.StaticType
import org.partiql.lang.types.UnknownArguments

internal const val DYNAMIC_LOOKUP_FUNCTION_NAME = "\$__dynamic_lookup__"

internal fun createBuiltinFunctionSignatures(): Map<String, FunctionSignature> =
// Creating a new IonSystem in this instance is not the problem it would normally be since we are
// discarding the created instances of the built-in functions after extracting all of the [FunctionSignature].
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,14 @@ import org.partiql.lang.ast.IsCountStarMeta
import org.partiql.lang.ast.passes.SemanticException
import org.partiql.lang.domains.PartiqlAst
import org.partiql.lang.domains.addSourceLocation
import org.partiql.lang.domains.errorContextFrom
import org.partiql.lang.errors.ErrorCode
import org.partiql.lang.errors.Property
import org.partiql.lang.errors.PropertyValueMap
import org.partiql.lang.eval.CompileOptions
import org.partiql.lang.eval.EvaluationException
import org.partiql.lang.eval.TypedOpBehavior
import org.partiql.lang.eval.err
import org.partiql.lang.eval.errorContextFrom
import org.partiql.pig.runtime.LongPrimitive

/**
Expand Down
50 changes: 50 additions & 0 deletions lang/src/org/partiql/lang/planner/GlobalBindings.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
package org.partiql.lang.planner

import org.partiql.lang.eval.BindingCase
import org.partiql.lang.eval.BindingName

/** Indicates the result of an attempt to resolve a global binding. */
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand we have kdoc for GlobalBindings.resolve() but can we also add a definition for Global Binding here?

BTW.
I think it's helpful to have the link to the upcoming documentation in applicable constructs under planner.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have rephrased this to be more clear.

sealed class ResolutionResult {
/**
* A success case, indicates the [uniqueId] of the match to the [BindingName] in the global scope.
* Typically, this is defined by the storage layer.
*/
data class GlobalVariable(val uniqueId: String) : ResolutionResult()

/**
* A success case, indicates the [index] of the only possible match to the [BindingName] in a local lexical scope.
* This is `internal` because [index] is an implementation detail that shouldn't be accessible outside of this
* library.
*/
internal data class LocalVariable(val index: Int) : ResolutionResult()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Considering that GlobalBindings.resolve() is meant to return GlobalVariable or undefined, should this be here?

Copy link
Member Author

@dlurton dlurton May 13, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's internal, so it's not exposed to customers. Customer code is never intended to resolve local variables, so this fits the situation. Having this here does simplify a few things in relation in the LogicalToLogicalResolvedVisitorTransform class. Namely, I found this to be preferable to having two variations of this sealed class: one that is public and only has GlobalVariable and Undefined, and another that is private and has GlobalVariable, Undefined as well as LocalVariable. There would also need to be logic to convert from the public variation to the private.

Making one member of a public sealed class internal is perhaps a little unusual, but unusual doesn't equate to undesirable, necessarily. Was there a specific concern you had with this approach?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wasn't worried about exposing it, I just thought this data class doesn't belong here. I think your explanation makes sense, so let's leave it as is.


/** A failure case, indicates that resolution did not match any variable. */
object Undefined : ResolutionResult()
}

fun interface GlobalBindings {
lziq marked this conversation as resolved.
Show resolved Hide resolved
/**
* Implementations try to resolve a global variable which is typically a database table, as identified by a
* [bindingName]. The [bindingName] includes both the name as specified by the query author and a [BindingCase]
* which indicates if query author included double quotes (") which mean the lookup should be case-sensitive.
*
* Implementations of this function must return:
*
* - [ResolutionResult.GlobalVariable] if [bindingName] matches a global variable (typically a database table).
* - [ResolutionResult.Undefined] if no identifier matches [bindingName].
*
* When determining if a variable name matches a global variable, it is important to consider if the comparison
* should be case-sensitive or case-insensitive. @see [BindingName.bindingCase]. In the event that more than one
* variable matches a case-insensitive [BindingName], the implementation must still select one of them
* without providing an error. (This is consistent with Postres's behavior in this scenario.)
*
* Note that while [ResolutionResult.LocalVariable] exists, it is intentionally marked `internal` and cannot
* be used by outside of this project..
*/
fun resolve(bindingName: BindingName): ResolutionResult
}

private val EMPTY = GlobalBindings { ResolutionResult.Undefined }

/** Convenience function for obtaining an instance of [GlobalBindings] with no defined variables. */
fun emptyGlobalBindings(): GlobalBindings = EMPTY
15 changes: 15 additions & 0 deletions lang/src/org/partiql/lang/planner/PassResult.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package org.partiql.lang.planner
import org.partiql.lang.errors.Problem

sealed class PassResult<TResult> {
/**
* Indicates query planning was successful and includes a list of any warnings that were encountered along the way.
*/
data class Success<TResult>(val result: TResult, val warnings: List<Problem>) : PassResult<TResult>()

/**
* Indicates query planning was not successful and includes a list of errors and warnings that were encountered
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

errors and warnings -> errors?

Copy link
Member Author

@dlurton dlurton May 13, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is designed to allow us to continue at least the current pass in the event of a semantic error instead of throwing an exception. That means its entirely possible to encounter both warnings and errors. I'll add a note.

* along the way.
*/
data class Error<TResult>(val errors: List<Problem>) : PassResult<TResult>()
}
25 changes: 25 additions & 0 deletions lang/src/org/partiql/lang/planner/transforms/AstNormalize.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
package org.partiql.lang.planner.transforms

import org.partiql.lang.domains.PartiqlAst
import org.partiql.lang.eval.visitors.FromSourceAliasVisitorTransform
import org.partiql.lang.eval.visitors.PipelinedVisitorTransform
import org.partiql.lang.eval.visitors.SelectListItemAliasVisitorTransform
import org.partiql.lang.eval.visitors.SelectStarVisitorTransform

/**
* Executes the [SelectListItemAliasVisitorTransform], [FromSourceAliasVisitorTransform] and
* [SelectStarVisitorTransform] passes on the receiver.
*/
fun PartiqlAst.Statement.normalize(): PartiqlAst.Statement {
// Since these passes all work on PartiqlAst, we can use a PipelinedVisitorTransform which executes each
// specified VisitorTransform in sequence.
val transforms = PipelinedVisitorTransform(
// Synthesizes unspecified `SELECT <expr> AS ...` aliases
SelectListItemAliasVisitorTransform(),
// Synthesizes unspecified `FROM <expr> AS ...` aliases
FromSourceAliasVisitorTransform(),
// Changes `SELECT * FROM a, b` to SELECT a.*, b.* FROM a, b`
SelectStarVisitorTransform()
)
return transforms.transformStatement(this)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,167 @@
package org.partiql.lang.planner.transforms

import org.partiql.lang.domains.PartiqlAst
import org.partiql.lang.domains.PartiqlAstToPartiqlLogicalVisitorTransform
import org.partiql.lang.domains.PartiqlLogical

/**
* Transforms an instance of [PartiqlAst.Statement] to [PartiqlLogical.Statement].
*
* Performs no semantic checks.
*
* This conversion (and the logical algebra) are early in their lifecycle and so only a very limited subset of
* SFW queries are transformable. See tests for this class to see which queries are transformable.
*/
internal fun PartiqlAst.Statement.toLogicalPlan(): PartiqlLogical.Plan =
PartiqlLogical.build {
plan(
AstToLogicalVisitorTransform.transformStatement(this@toLogicalPlan),
version = PLAN_VERSION_NUMBER.toLong()
)
}

private object AstToLogicalVisitorTransform : PartiqlAstToPartiqlLogicalVisitorTransform() {

override fun transformExprSelect(node: PartiqlAst.Expr.Select): PartiqlLogical.Expr {
checkForUnsupportedSelectClauses(node)

var algebra: PartiqlLogical.Bexpr = FromSourceToBexpr.convert(node.from)

algebra = node.fromLet?.let { fromLet ->
PartiqlLogical.build {
let(algebra, fromLet.letBindings.map { transformLetBinding(it) }, node.fromLet.metas)
}
} ?: algebra

algebra = node.where?.let {
PartiqlLogical.build { filter(transformExpr(it), algebra, it.metas) }
} ?: algebra

algebra = node.offset?.let {
PartiqlLogical.build { offset(transformExpr(it), algebra, node.offset.metas) }
} ?: algebra

algebra = node.limit?.let {
PartiqlLogical.build { limit(transformExpr(it), algebra, node.limit.metas) }
} ?: algebra

return convertProjectionToBindingsToValues(node, algebra)
}

private fun convertProjectionToBindingsToValues(node: PartiqlAst.Expr.Select, algebra: PartiqlLogical.Bexpr) =
PartiqlLogical.build {
bindingsToValues(
when (val project = node.project) {
is PartiqlAst.Projection.ProjectValue -> transformExpr(project.value)
is PartiqlAst.Projection.ProjectList -> {
struct(
List(project.projectItems.size) { idx ->
when (val projectItem = project.projectItems[idx]) {
is PartiqlAst.ProjectItem.ProjectExpr ->
structField(
lit(
projectItem.asAlias?.toIonElement()
?: errAstNotNormalized("SELECT-list item alias not specified")
),
transformExpr(projectItem.expr),
)
is PartiqlAst.ProjectItem.ProjectAll -> {
structFields(transformExpr(projectItem.expr), projectItem.metas)
}
}
}
)
}
is PartiqlAst.Projection.ProjectStar ->
// `SELECT * FROM bar AS b` is rewritten to `SELECT b.* FROM bar as b` by
// [SelectStarVisitorTransform]. Therefore, there is no need to support `SELECT *` here.
errAstNotNormalized("Expected SELECT * to be removed")

is PartiqlAst.Projection.ProjectPivot -> TODO("PIVOT ...")
},
algebra,
node.project.metas
)
}.let { q ->
// in case of SELECT DISTINCT, wrap bindingsToValues in call to filter_distinct
when (node.setq) {
null, is PartiqlAst.SetQuantifier.All -> q
is PartiqlAst.SetQuantifier.Distinct -> PartiqlLogical.build { call("filter_distinct", q) }
}
}

/**
* Throws [NotImplementedError] if any `SELECT` clauses were used that are not mappable to [PartiqlLogical].
*
* This function is temporary and will be removed when all the clauses of the `SELECT` expression are mappable
* to [PartiqlLogical].
*/
private fun checkForUnsupportedSelectClauses(node: PartiqlAst.Expr.Select) {
when {
node.group != null -> TODO("Support for GROUP BY")
node.order != null -> TODO("Support for ORDER BY")
node.having != null -> TODO("Support for HAVING")
}
}

override fun transformLetBinding(node: PartiqlAst.LetBinding): PartiqlLogical.LetBinding =
PartiqlLogical.build {
letBinding(
transformExpr(node.expr),
varDecl_(node.name, node.name.metas),
node.metas
)
}

override fun transformStatementDml(node: PartiqlAst.Statement.Dml): PartiqlLogical.Statement {
TODO("Support for DML")
}

override fun transformStatementDdl(node: PartiqlAst.Statement.Ddl): PartiqlLogical.Statement {
TODO("Support for DDL")
}
lziq marked this conversation as resolved.
Show resolved Hide resolved

override fun transformExprStruct(node: PartiqlAst.Expr.Struct): PartiqlLogical.Expr =
PartiqlLogical.build {
struct(
node.fields.map {
structField(
transformExpr(it.first),
transformExpr(it.second)
)
},
metas = node.metas
)
}
}

private object FromSourceToBexpr : PartiqlAst.FromSource.Converter<PartiqlLogical.Bexpr> {

override fun convertScan(node: PartiqlAst.FromSource.Scan): PartiqlLogical.Bexpr {
val asAlias = node.asAlias ?: errAstNotNormalized("Expected as alias to be non-null")
return PartiqlLogical.build {
scan(
AstToLogicalVisitorTransform.transformExpr(node.expr),
varDecl_(asAlias, asAlias.metas),
node.atAlias?.let { varDecl_(it, it.metas) },
node.byAlias?.let { varDecl_(it, it.metas) },
node.metas
)
}
}

override fun convertUnpivot(node: PartiqlAst.FromSource.Unpivot): PartiqlLogical.Bexpr {
TODO("Support for UNPIVOT")
}

override fun convertJoin(node: PartiqlAst.FromSource.Join): PartiqlLogical.Bexpr =
PartiqlLogical.build {
join(
joinType = AstToLogicalVisitorTransform.transformJoinType(node.type),
left = convert(node.left),
right = convert(node.right),
predicate = node.predicate?.let { AstToLogicalVisitorTransform.transformExpr(it) },
node.metas
)
}
}
Loading