Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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 @@ -4,7 +4,7 @@
import org.partiql.plan.Operand;

/**
* Match represents a subtree match to be sent to the
* Match represents a subtree match to be sent to the {@link Strategy}.
*/
public class Match {

Expand Down
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