Skip to content
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
4 changes: 2 additions & 2 deletions partiql-cli/src/main/kotlin/org/partiql/cli/Main.kt
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import org.partiql.spi.catalog.Catalog
import org.partiql.spi.catalog.Name
import org.partiql.spi.catalog.Session
import org.partiql.spi.catalog.Table
import org.partiql.spi.errors.PErrorException
import org.partiql.spi.errors.PRuntimeException
import org.partiql.spi.value.Datum
import org.partiql.spi.value.DatumReader
import org.partiql.spi.value.ValueUtils
Expand Down Expand Up @@ -210,7 +210,7 @@ internal class MainCommand : Runnable {
val writer = PartiQLValueTextWriter(System.out)
val p = ValueUtils.newPartiQLValue(result)
writer.append(p) // TODO: Create a Datum writer
} catch (e: PErrorException) {
} catch (e: PRuntimeException) {
val msg = ErrorMessageFormatter.message(e.error)
error(msg)
}
Expand Down
14 changes: 9 additions & 5 deletions partiql-cli/src/main/kotlin/org/partiql/cli/pipeline/Pipeline.kt
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,10 @@ import org.partiql.plan.Plan
import org.partiql.planner.PartiQLPlanner
import org.partiql.spi.Context
import org.partiql.spi.catalog.Session
import org.partiql.spi.errors.PErrorException
import org.partiql.spi.errors.PErrorListenerException
import org.partiql.spi.errors.PError
import org.partiql.spi.errors.PErrorKind
import org.partiql.spi.errors.PRuntimeException
import org.partiql.spi.errors.Severity
import org.partiql.spi.value.Datum
import java.io.PrintStream

Expand All @@ -24,7 +26,7 @@ internal class Pipeline private constructor(

/**
* TODO replace with the ResultSet equivalent?
* @throws PipelineException when there are accumulated errors, or if the components have thrown an [PErrorListenerException].
* @throws PipelineException when there are accumulated errors, or if the components have thrown an [PRuntimeException].
*/
@Throws(PipelineException::class)
fun execute(statement: String, session: Session): Datum {
Expand Down Expand Up @@ -65,7 +67,7 @@ internal class Pipeline private constructor(
action.invoke()
} catch (e: PipelineException) {
throw e
} catch (e: PErrorException) {
} catch (e: PRuntimeException) {
val message = ErrorMessageFormatter.message(e.error)
throw PipelineException(message)
}
Expand Down Expand Up @@ -98,7 +100,9 @@ internal class Pipeline private constructor(
/**
* Halts execution.
*/
class PipelineException(override val message: String?) : PErrorListenerException(message)
class PipelineException(override val message: String?) : PRuntimeException(
PError(PError.INTERNAL_ERROR, Severity.ERROR(), PErrorKind.EXECUTION(), null, null)
)

/**
* Configuration for passing through user-defined configurations to the underlying components.
Expand Down
4 changes: 2 additions & 2 deletions partiql-cli/src/main/kotlin/org/partiql/cli/shell/Shell.kt
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ import org.joda.time.Duration
import org.partiql.cli.pipeline.ErrorMessageFormatter
import org.partiql.cli.pipeline.Pipeline
import org.partiql.spi.catalog.Session
import org.partiql.spi.errors.PErrorException
import org.partiql.spi.errors.PRuntimeException
import org.partiql.spi.value.ValueUtils
import org.partiql.spi.value.io.PartiQLValueTextWriter
import java.io.Closeable
Expand Down Expand Up @@ -280,7 +280,7 @@ internal class Shell(
val writer = PartiQLValueTextWriter(out)
val p = ValueUtils.newPartiQLValue(result)
writer.append(p) // TODO: Create a Datum writer
} catch (e: PErrorException) {
} catch (e: PRuntimeException) {
val message = ErrorMessageFormatter.message(e.error)
out.error(message)
}
Expand Down
6 changes: 3 additions & 3 deletions partiql-eval/api/partiql-eval.api
Original file line number Diff line number Diff line change
Expand Up @@ -21,20 +21,20 @@ public abstract interface class org/partiql/eval/ExprValue : org/partiql/eval/Ex
public abstract fun eval (Lorg/partiql/eval/Environment;)Lorg/partiql/spi/value/Datum;
}

public class org/partiql/eval/Mode {
public class org/partiql/eval/Mode : org/partiql/spi/Enum {
public static final field PERMISSIVE I
public static final field STRICT I
public static fun PERMISSIVE ()Lorg/partiql/eval/Mode;
public static fun STRICT ()Lorg/partiql/eval/Mode;
public fun code ()I
public fun name ()Ljava/lang/String;
}

public class org/partiql/eval/Row {
public final field values [Lorg/partiql/spi/value/Datum;
public fun <init> ()V
public fun <init> ([Lorg/partiql/spi/value/Datum;)V
public fun concat (Lorg/partiql/eval/Row;)Lorg/partiql/eval/Row;
public fun equals (Ljava/lang/Object;)Z
public final fun getValues ()[Lorg/partiql/spi/value/Datum;
public fun hashCode ()I
public static fun of ([Lorg/partiql/spi/value/Datum;)Lorg/partiql/eval/Row;
public fun toString ()Ljava/lang/String;
Expand Down
4 changes: 3 additions & 1 deletion partiql-eval/src/main/java/org/partiql/eval/Environment.java
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ private Environment(Row[] stack) {

/**
* Push a new row onto the stack.
* @param row the row to push
* @return the new environment
*/
public Environment push(Row row) {
int n = stack.length;
Expand All @@ -59,7 +61,7 @@ public Environment push(Row row) {
*/
public Datum get(int depth, int offset) {
try {
return stack[depth].values[offset];
return stack[depth].getValues()[offset];
} catch (IndexOutOfBoundsException ex) {
throw new RuntimeException("Invalid variable reference [$depth:$offset]\n$this");
}
Expand Down
11 changes: 11 additions & 0 deletions partiql-eval/src/main/java/org/partiql/eval/ExprRelation.java
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,24 @@
*/
public interface ExprRelation extends Expr, AutoCloseable, Iterator<Row> {

/**
* Prepares all resources for execution. Resets any modified state.
* @param env the environment to use for evaluation
*/
public void open(@NotNull Environment env);

@NotNull
public Row next();

/**
* Returns true if there are more rows to be returned.
* @return true if there are more rows to be returned
*/
public boolean hasNext();

/**
* Closes and resets all resources used for execution.
*/
public void close();

@Override
Expand Down
35 changes: 26 additions & 9 deletions partiql-eval/src/main/java/org/partiql/eval/Mode.java
Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
package org.partiql.eval;

import org.jetbrains.annotations.NotNull;
import org.partiql.spi.Enum;
import org.partiql.spi.UnsupportedCodeException;

/**
* PartiQL Execution Mode.
*/
public class Mode {
public class Mode extends Enum {

/**
* Strict execution mode.
Expand All @@ -15,23 +19,36 @@ public class Mode {
*/
public static final int PERMISSIVE = 1;

/**
* Internal enum code.
*/
private final int code;

private Mode(int code) {
this.code = code;
super(code);
}

public int code() {
return this.code;
@NotNull
@Override
public String name() throws UnsupportedCodeException {
int code = code();
switch (code) {
case STRICT:
return "STRICT";
case PERMISSIVE:
return "PERMISSIVE";
default:
throw new UnsupportedCodeException(code);
}
}

/**
* Returns the {@link Mode#STRICT} execution mode.
* @return the {@link Mode#STRICT} execution mode
*/
public static Mode STRICT() {
return new Mode(STRICT);
}

/**
* Returns the {@link Mode#PERMISSIVE} execution mode.
* @return the {@link Mode#PERMISSIVE} execution mode
*/
public static Mode PERMISSIVE() {
return new Mode(PERMISSIVE);
}
Expand Down
12 changes: 8 additions & 4 deletions partiql-eval/src/main/java/org/partiql/eval/Row.java
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,18 @@
*/
public class Row {

private final Datum[] values;

/**
* TODO internalize values.
* Get the values in this record.
* @return the values
*/
public final Datum[] values;
public final Datum[] getValues() {
return values;
}

/**
* TODO keep ??
*
* Create a record with the given values.
* @param values the values
* @return the record
*/
Expand Down
10 changes: 7 additions & 3 deletions partiql-eval/src/main/java/org/partiql/eval/Statement.java
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package org.partiql.eval;

import org.jetbrains.annotations.NotNull;
import org.partiql.spi.errors.PRuntimeException;
import org.partiql.spi.value.Datum;

/**
Expand All @@ -9,10 +10,13 @@
public interface Statement {

/**
* Executes the prepared statement.
*
* Converts the compiled statement into a (sometimes lazily-evaluated) {@link Datum} that can be used to execute
* the statement. For scalar expressions, this is typically materialized immediately. For non-scalar expressions,
* this may be materialized lazily. Consumers of this API should be aware that the returned {@link Datum} may be
* evaluated multiple times. Please see more documentation on <a href="https://www.partiql.org">the PartiQL website</a>.
* @return Datum execution result.
* @throws PRuntimeException if an error was encountered during execution
*/
@NotNull
public Datum execute();
public Datum execute() throws PRuntimeException;
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import org.partiql.eval.internal.compiler.StandardCompiler;
import org.partiql.plan.Plan;
import org.partiql.spi.Context;
import org.partiql.spi.errors.PRuntimeException;

import java.util.ArrayList;
import java.util.List;
Expand All @@ -19,33 +20,45 @@ public interface PartiQLCompiler {
* Prepares the given plan into an executable PartiQL statement.
*
* @param plan The plan to compile.
* @param mode The mode to use when compiling the plan.
* @return The prepared statement.
* @throws PRuntimeException If an error occurs during compilation.
*/
@NotNull
default Statement prepare(@NotNull Plan plan, @NotNull Mode mode) {
default Statement prepare(@NotNull Plan plan, @NotNull Mode mode) throws PRuntimeException {
return prepare(plan, mode, Context.standard());
}

/**
* Prepares the given plan into an executable PartiQL statement.
*
* @param plan The plan to compile.
* @param mode The mode to use when compiling the plan.
* @param ctx The context to use when compiling the plan.
* @throws PRuntimeException If an error occurs during compilation. The error might have been emitted by the {@code ctx}'s {@link Context#getErrorListener()}.
* @return The prepared statement.
*/
@NotNull
public Statement prepare(@NotNull Plan plan, @NotNull Mode mode, @NotNull Context ctx);
public Statement prepare(@NotNull Plan plan, @NotNull Mode mode, @NotNull Context ctx) throws PRuntimeException;

/**
* @return A new [PartiQLCompilerBuilder].
* Returns a new {@link Builder}.
* @return a new {@link Builder}.
*/
@NotNull
public static Builder builder() {
return new Builder();
}

/**
* @return A new [PartiQLCompiler].
* @return a new {@link PartiQLCompiler}.
*/
public static PartiQLCompiler standard() {
return new StandardCompiler();
}

/**
* Builder class for the [PartiQLCompiler] interface.
* Builder class for the {@link PartiQLCompiler} interface.
*/
public static class Builder {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,12 @@ protected Pattern(@NotNull Class<? extends Operator> clazz, @Nullable Predicate<
this.predicate = predicate;
}

/**
* Matches an operator against this pattern.
*
* @param operator Operator to match.
* @return true if the operator matches the pattern.
*/
public boolean matches(Operator operator) {
if (!clazz.isInstance(operator)) {
return false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ public Strategy(@NotNull Pattern pattern) {
}

/**
* Get the pattern associated with this strategy.
* @return the pattern associated with this strategy
*/
@NotNull
Expand All @@ -48,6 +49,8 @@ public Pattern getPattern() {
public interface Callback {

/**
* Compiles the given logical operator into a physical operator.
* @param operator the logical operator to compile
* @return a physical operator (expr) for the logical operator.
*/
@NotNull
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import org.partiql.eval.Statement
import org.partiql.eval.compiler.Match
import org.partiql.eval.compiler.PartiQLCompiler
import org.partiql.eval.compiler.Strategy
import org.partiql.eval.internal.helpers.PErrors
import org.partiql.eval.internal.operator.Aggregate
import org.partiql.eval.internal.operator.rel.RelOpAggregate
import org.partiql.eval.internal.operator.rel.RelOpDistinct
Expand Down Expand Up @@ -110,7 +111,7 @@ import org.partiql.plan.rex.RexVar
import org.partiql.spi.Context
import org.partiql.spi.errors.PError
import org.partiql.spi.errors.PErrorKind
import org.partiql.spi.errors.PErrorListenerException
import org.partiql.spi.errors.PRuntimeException
import org.partiql.spi.types.PType
import org.partiql.spi.value.Datum

Expand All @@ -132,7 +133,7 @@ internal class StandardCompiler(strategies: List<Strategy>) : PartiQLCompiler {
else -> throw IllegalArgumentException("Only query statements are supported")
}
return statement
} catch (e: PErrorListenerException) {
} catch (e: PRuntimeException) {
throw e
} catch (t: Throwable) {
val error = PError.INTERNAL_ERROR(PErrorKind.COMPILATION(), null, t)
Expand All @@ -158,7 +159,15 @@ internal class StandardCompiler(strategies: List<Strategy>) : PartiQLCompiler {
private val root = compile(action.getRex(), Unit).catch()

// execute with no parameters
override fun execute(): Datum = root.eval(Environment())
override fun execute(): Datum {
return try {
root.eval(Environment())
} catch (e: PRuntimeException) {
throw e
} catch (t: Throwable) {
throw PErrors.internalErrorException(t)
}
Comment on lines +167 to +169
Copy link
Member

Choose a reason for hiding this comment

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

I haven't checked this locally but shouldn't the non-PRuntimeExceptions be caught? I guess not a big deal to add an additional catch just in case.

Copy link
Member Author

Choose a reason for hiding this comment

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

Wdym? We are catching Throwable here

Copy link
Member

Choose a reason for hiding this comment

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

To rephrase, I was initially thinking that all the Throwable exceptions were already wrapped in a PRuntimeException, but seems like we need this additional catch in case there's some exception that we haven't already wrapped in a PRuntimeException. For example (this specific case has been fixed by #1720), 1.0 / 0.0 would give a "java.lang.ArithmeticException: / by zero" which would be wrapped in a PRuntimeException with code PError.INTERNAL_ERROR rather than a PRuntimeException with code PError.DIVISION_BY_ZERO. So the following two cases would give different error codes:

1 / 0 -- division by zero error
1.0 / 0.0 -- internal error

I think this catch(throwable) is fine for now, but we should make sure exceptions we throw try to use the existing PRuntimeException codes.

}
}

/**
Expand Down
Loading
Loading