[Pljava-dev] possible patch for cancelling a trigger operation

Dear Pl/Java users and developers,
please find attached a minimal patch for cancelling a trigger operation. I
attach also a simple Java file that provides a trigger function that counts
each insert/update operations and will allow only even operations. The trigger
can be used against any table once the jar is installed in the database as
follows:

CREATE FUNCTION trigger_test() RETURNS trigger AS
'itpug.pljava.CancelStatementTriggerTest.triggerFunction' LANGUAGE 'java';

CREATE TRIGGER trigger_test_impl BEFORE INSERT OR UPDATE OR DELETE ON
java_table FOR
EACH ROW EXECUTE PROCEDURE trigger_test();

(substitute java_table with your own table - be aware that the trigger will
print a message on the logs).

I've tested the patch against the following system:
- PostgreSQL 8.4.4 on x86_64-pc-linux-gnu, compiled by GCC gcc-4.4.real
(Ubuntu 4.4.3-4ubuntu5) 4.4.3, 64-bit
- the latest version of the pljava sources
- jdk 1.5.0_22 on Ubuntu 64 bit
- gcc 4.4.3

The patch works as follows:
0) since TriggerData is the only way to interact with the backed, the logic of
cancelling the current insert/update/delete statements must pass thru the
TriggerData class. To allows this I've inserted two methods to set the current
statement as cancelled and to know if the current statement has been
cancelled.
1) the internal/TriggerData class now implements such methods using a simple
boolean flag to indicate if the current statement must be cancelled or not.
Please note that there is a utility method to check the conditions on which a
statement can be aborted.
2) the internal/TriggerData class when returning the tuple to apply, simply
returns 0 if the statement has been cancelled. Such 0 is interpreted as a NULL
result in the backend code (Functions.c) and therefore the trigger aborts the
current operation.
3) when asking for a newResultSet on TriggerData, the system checks if the
trigger has already been marked for cancelling the current statement, so that
the returned result set is set read-only. This is not the optimal solution,
since one can get the result set before cancelling the trigger, but in any
case the getTriggerResultTuple will avoid insertions/updates.

I've also inserted a few comments while studying the call flow in order to
better understand.

Hope this can be applied as a stable patch, in such case I've other ideas to
better improve the patch on which I can work (e.g., propagate the status to
the ResultSets, use a static flag instead of an instance one, etc.).

Luca
-------------- next part --------------
diff --git a/src/C/pljava/Function.c b/src/C/pljava/Function.c
index 434d9a9..b8012da 100644
--- a/src/C/pljava/Function.c
+++ b/src/C/pljava/Function.c
@@ -755,12 +755,26 @@ Datum Function_invoke(Function self, PG_FUNCTION_ARGS)
return retVal;
}

+
+/*
+ * This function is called from the backend.c when a trigger must execute, that means when a user-defined
+ * function must be executed as a trigger function.
+ * As for any backend-trigger function written in C, this function receives the PG_FUNCTION_ARGS that
+ * defines the user-defined function parameters. Moreover, the function receives the Function "self" pointer
+ * that is a struct that contains information about the Java method to call, the result type, if the function
+ * is a multi-call one (i.e., it requires/it will allocate a memory context) and so on.
+ */
Datum Function_invokeTrigger(Function self, PG_FUNCTION_ARGS)
{
jvalue arg;
Datum ret;

+ // create a trigger data object starting from the current trigger data passed from
+ // the backend to the function (remember that context here is the trigger data).
+ // This method simply creates a Java object starting from the C TriggerData one.
arg.l = TriggerData_create((TriggerData*)fcinfo->context);
+
+ // if the trigger data cannot be created than the trigger must abort (i.e., return null)
if(arg.l == 0)
return 0;

@@ -769,7 +783,8 @@ Datum Function_invokeTrigger(Function self, PG_FUNCTION_ARGS)

fcinfo->isnull = false;
if(JNI_exceptionCheck())
- ret = 0;
+ // exception, so return null (i.e., abort the trigger)
+ ret = 0;
else
{
/* A new Tuple may or may not be created here. If it is, ensure that
@@ -778,9 +793,15 @@ Datum Function_invokeTrigger(Function self, PG_FUNCTION_ARGS)
MemoryContext currCtx = Invocation_switchToUpperContext();
ret = PointerGetDatum(TriggerData_getTriggerReturnTuple(arg.l, &fcinfo->isnull));

- /* Triggers are not allowed to set the fcinfo->isnull, even when
+ /*
+ * Triggers are not allowed to set the fcinfo->isnull, even when
* they return null.
- */
+ * In fact fcinfo->isnull means that the function is returning a NULL SQL value, while a
+ * trigger returning null means that the current statement must be cancelled, not that
+ * the trigger is returning a NULL SQL value.
+ * Please note that the method TriggerData.getTriggerReturnTuple will set the fcinfo->isnull
+ * value according to the trigger function returning NULL, but this is not useful here.
+ */
fcinfo->isnull = false;

MemoryContextSwitchTo(currCtx);
diff --git a/src/C/pljava/type/TriggerData.c b/src/C/pljava/type/TriggerData.c
index 1d65bc3..95359d5 100644
--- a/src/C/pljava/type/TriggerData.c
+++ b/src/C/pljava/type/TriggerData.c
@@ -35,7 +35,11 @@ HeapTuple TriggerData_getTriggerReturnTuple(jobject jtd, bool* wasNull)
{
Ptr2Long p2l;
HeapTuple ret = 0;
- p2l.longVal = JNI_callLongMethod(jtd, s_TriggerData_getTriggerReturnTuple);
+
+ // call the method identified by the "s_triggerData_getTriggerReturnTuple" id on the
+ // object "jtd", that is a Java instance of the TriggerData
+ p2l.longVal = JNI_callLongMethod(jtd, // java TriggerData
+ s_TriggerData_getTriggerReturnTuple); // method id to call
if(p2l.longVal != 0)
ret = heap_copytuple((HeapTuple)p2l.ptrVal);
else
diff --git a/src/java/pljava/org/postgresql/pljava/TriggerData.java b/src/java/pljava/org/postgresql/pljava/TriggerData.java
index 78aa87c..8cca8d9 100644
--- a/src/java/pljava/org/postgresql/pljava/TriggerData.java
+++ b/src/java/pljava/org/postgresql/pljava/TriggerData.java
@@ -144,4 +144,24 @@ public interface TriggerData
* if the contained native buffer has gone stale.
*/
boolean isFiredByUpdate() throws SQLException;
+
+
+ /**
+ * Sets the <code>aborted</code> flag of this TriggerData. A TriggerData aborted
+ * will return an empty result set, so that the trigger will abort its execution and
+ * the insertion/update will be aborted too.
+ * Please note that this can be set only for a trigger invoked before each row and for
+ * insert/update.
+ */
+ public void cancelCurrentStatement( boolean abort ) throws SQLException;
+
+
+ /**
+ * Is this trigger aborted?
+ */
+ public boolean isCurrentStatementCancelled() throws SQLException;
+
+
+
+
}
diff --git a/src/java/pljava/org/postgresql/pljava/internal/TriggerData.java b/src/java/pljava/org/postgresql/pljava/internal/TriggerData.java
index da9c16b..7aed3f8 100644
--- a/src/java/pljava/org/postgresql/pljava/internal/TriggerData.java
+++ b/src/java/pljava/org/postgresql/pljava/internal/TriggerData.java
@@ -34,9 +34,12 @@ public class TriggerData extends JavaWrapper implements org.postgresql.pljava.Tr
* be null for delete triggers and for triggers that was fired for
* statement. <br/>The returned set will be updateable and positioned on a
* valid row.
+ * If the current statement has been cancelled the returned result set will be
+ * read-only.
*
- * @return An updateable <code>ResultSet</code> containing one row or
- * <code>null</code>.
+ * @return An updateable <code>ResultSet</code> containing one row (if the trigger has been invoked for each row, before and the
+ * current statement has not been cancelled), a read-only result set in the case of an after trigger (or if the
+ * current statement has been cancelled) or <code>null</code>.
* @throws SQLException
* if the contained native buffer has gone stale.
*/
@@ -55,9 +58,8 @@ public class TriggerData extends JavaWrapper implements org.postgresql.pljava.Tr
? this.getTriggerTuple()
: this.getNewTuple();

- // Triggers fired after will always have a read-only row
- //
- m_new = new TriggerResultSet(this.getRelation().getTupleDesc(), tuple, this.isFiredAfter());
+ // Triggers fired after will always have a read-only row, and so do triggers that have been cancelled.
+ m_new = new TriggerResultSet(this.getRelation().getTupleDesc(), tuple, (this.isFiredAfter() | this.isCurrentStatementCancelled()) );
return m_new;
}

@@ -66,6 +68,9 @@ public class TriggerData extends JavaWrapper implements org.postgresql.pljava.Tr
* be null for insert triggers and for triggers that was fired for
* statement. <br/>The returned set will be read-only and positioned on a
* valid row.
+ *
+ * Please note that a result set will be returned even if the current statement is cancelled, this is not
+ * a security hole since the result set will be read-only!
*
* @return A read-only <code>ResultSet</code> containing one row or
* <code>null</code>.
@@ -77,12 +82,69 @@ public class TriggerData extends JavaWrapper implements org.postgresql.pljava.Tr
if (m_old != null)
return m_old;

- if (this.isFiredByInsert() || this.isFiredForStatement())
+ if (this.isFiredByInsert() || this.isFiredForStatement() )
return null;
m_old = new TriggerResultSet(this.getRelation().getTupleDesc(), this.getTriggerTuple(), true);
return m_old;
}

+
+
+ /**
+ * This flag indicates if the trigger must be aborted, that is if the trigger must return a null tuple
+ * so to abort the current operation. This flag is not made static in order to allow the trigger to start from a clean
+ * value for each row. If this is made static, than the trigger status will be kept along invocations of the same trigger function.
+ */
+ private boolean aborted = false;
+
+ /**
+ * Sets the <code>aborted</code> flag of this TriggerData. A TriggerData aborted
+ * will return an empty result set, so that the trigger will abort its execution and
+ * the insertion/update will be aborted too.
+ * Please note that this can be set only for a trigger invoked before each row and for
+ * insert/update/delete.
+ */
+ public final synchronized void cancelCurrentStatement( boolean abort ) throws SQLException{
+ // check that this is called only on a insert/update trigger for before events
+ if( ! this.canCancelStatement() )
+ throw new SQLException("Cannot abort a trigger not handling a before event for inserts/updates!");
+
+ this.aborted = abort;
+ }
+
+ /**
+ * Is this trigger aborted?
+ */
+ public final synchronized boolean isCurrentStatementCancelled() throws SQLException{
+ // security check
+ if( ! this.canCancelStatement() )
+ this.aborted = false;
+
+
+ return this.aborted;
+ }
+
+
+ /**
+ * An utility method to test if the current trigger can cancel the current statement, that means
+ * if it can delete the current in-progress operation (for instance INSERT command).
+ * A trigger can cancel the current statement only if (and logical conditions):
+ * - it is invoked for a before event
+ * - it is invoked on each row
+ * - it is invoked for an insert/update/delete
+ *
+ * and this means that it cannot cancel the statement if (or logical conditions):
+ * - it is invoked for an after event
+ * - it is invoked for a statement
+ */
+ private final boolean canCancelStatement() throws SQLException{
+ if( this.isFiredAfter() || this.isFiredForStatement() )
+ return false;
+ else
+ return true;
+ }
+
+
/**
* Commits the changes made on the <code>ResultSet</code> representing
* <code>new</code> and returns the native pointer of new tuple. This
@@ -90,11 +152,12 @@ public class TriggerData extends JavaWrapper implements org.postgresql.pljava.Tr
* be called in any other way.
*
* @return The modified tuple, or if no modifications have been made, the
- * original tuple.
+ * original tuple. In the case no one tuple must be returned, than
+ * this method will return a zero value.
*/
public long getTriggerReturnTuple() throws SQLException
{
- if(this.isFiredForStatement() || this.isFiredAfter())
+ if(this.isFiredForStatement() || this.isFiredAfter() || this.isCurrentStatementCancelled() )
//
// Only triggers fired before each row can have a return
// value.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: CancelStatementTriggerTest.java
Type: text/x-java
Size: 1303 bytes
Desc: not available
URL: <http://lists.pgfoundry.org/pipermail/pljava-dev/attachments/20100621/20fd76ad/attachment.bin>