Skip to content

Commit

Permalink
Improve transaction handling in Prism Driver & Decimal validation (#514)
Browse files Browse the repository at this point in the history
  • Loading branch information
gartens authored Oct 19, 2024
1 parent 00c0522 commit 5da9c7e
Show file tree
Hide file tree
Showing 21 changed files with 162 additions and 59 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ public String toString() {

@Override
public int compareTo( CorrelationId other ) {
return id - other.id;
return Integer.compare( id, other.id );
}


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2137,7 +2137,7 @@ public static PolyBoolean isNotFalse( PolyBoolean b ) {
* NULL → NULL, FALSE → TRUE, TRUE → FALSE.
*/
public static PolyBoolean not( PolyBoolean b ) {
return PolyBoolean.of( !b.value );
return b == null ? PolyBoolean.of( null ) : PolyBoolean.of( !b.value );
}


Expand Down
21 changes: 9 additions & 12 deletions core/src/main/java/org/polypheny/db/languages/LanguageManager.java
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,11 @@ public List<ImplementationContext> anyPrepareQuery( QueryContext context, Statem
String changedNamespace = null;
for ( ParsedQueryContext parsed : parsedQueries ) {
if ( i != 0 ) {
// as long as we directly commit the transaction, we cannot reuse the same transaction
if ( previousDdl && !transaction.isActive() ) {
transaction = parsed.getTransactionManager().startTransaction( transaction.getUser().id, transaction.getDefaultNamespace().id, transaction.isAnalyze(), transaction.getOrigin() );
parsed.addTransaction( transaction );
}
statement = transaction.createStatement();
}
if ( changedNamespace != null ) {
Expand All @@ -155,22 +160,10 @@ public List<ImplementationContext> anyPrepareQuery( QueryContext context, Statem
new GenericRuntimeException( "DDL statement is not executable" ),
implementationContexts );
}
// as long as we directly commit the transaction, we cannot reuse the same transaction
if ( previousDdl && !transaction.isActive() ) {
transaction = parsed.getTransactionManager().startTransaction( transaction.getUser().id, transaction.getDefaultNamespace().id, transaction.isAnalyze(), transaction.getOrigin() );
statement = transaction.createStatement();
parsed.addTransaction( transaction );
}

implementation = processor.prepareDdl( statement, (ExecutableStatement) parsed.getQueryNode().get(), parsed );
previousDdl = true;
} else {
// as long as we directly commit the transaction, we cannot reuse the same transaction
if ( previousDdl && !transaction.isActive() ) {
transaction = parsed.getTransactionManager().startTransaction( transaction.getUser().id, transaction.getDefaultNamespace().id, transaction.isAnalyze(), transaction.getOrigin() );
statement = transaction.createStatement();
parsed.addTransaction( transaction );
}
previousDdl = false;
if ( parsed.getLanguage().validatorSupplier() != null ) {
if ( transaction.isAnalyze() ) {
Expand All @@ -195,6 +188,10 @@ public List<ImplementationContext> anyPrepareQuery( QueryContext context, Statem
if ( transaction.isAnalyze() ) {
statement.getOverviewDuration().stop( "Translation" );
}

if ( !statement.getTransaction().isActive() ) {
log.warn( "Transaction is not active" );
}
implementation = statement.getQueryProcessor().prepareQuery( root, true );
}
// queries are able to switch the context of the following queries
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -799,6 +799,8 @@ public boolean isNumber() {
public PolyNumber asNumber() {
if ( isNumber() ) {
return (PolyNumber) this;
} else if ( isString() ) {
return PolyFloat.convert( this.asString() );
}
throw cannotParse( this, PolyNumber.class );
}
Expand Down
2 changes: 1 addition & 1 deletion core/src/main/java/org/polypheny/db/util/ByteString.java
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ public int compareTo( ByteString that ) {
return c1 - c2;
}
}
return v1.length - v2.length;
return Integer.compare( v1.length, v2.length );
}


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,11 +83,11 @@ public int compare( Object o1, Object o2 ) {
if ( c != 0 ) {
return c;
}
if ( o1 instanceof Key ) {
return ((Key) o1).compareResult;
if ( o1 instanceof Key key ) {
return key.compareResult;
}
if ( o2 instanceof Key ) {
return -((Key) o2).compareResult;
if ( o2 instanceof Key key ) {
return -key.compareResult;
}
return s1.compareTo( s2 );
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ private PolyValue check( PolyValue value, AlgDataType type ) {

switch ( type.getPolyType() ) {
case DECIMAL -> {
if ( value.asNumber().toString().replace( ".", "" ).replace( "-", "" ).length() > type.getPrecision() ) {
if ( value.asNumber().toString().replaceFirst( "^0", "" ).replace( ".", "" ).replace( "-", "" ).length() > type.getPrecision() ) {
throw new GenericRuntimeException( "Numeric value is too long" );
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,9 @@ public void commit() throws TransactionException {
for ( Adapter<?> adapter : involvedAdapters ) {
adapter.commit( xid );
}
if ( involvedAdapters.isEmpty() ) {
log.debug( "No adapter used." );
}

this.statements.forEach( statement -> {
if ( statement.getMonitoringEvent() != null ) {
Expand Down Expand Up @@ -280,6 +283,9 @@ public Processor getProcessor( QueryLanguage language ) {

@Override
public StatementImpl createStatement() {
if ( !isActive() ) {
throw new IllegalStateException( "Transaction is not active!" );
}
StatementImpl statement = new StatementImpl( this );
statements.add( statement );
return statement;
Expand All @@ -289,7 +295,7 @@ public StatementImpl createStatement() {
@Override
public int compareTo( @NonNull Object o ) {
Transaction that = (Transaction) o;
return this.xid.hashCode() - that.getXid().hashCode();
return Integer.compare( this.xid.hashCode(), that.getXid().hashCode() );
}


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1160,4 +1160,67 @@ public void arrayBatchTest() throws SQLException {
}
}


@Test
public void transactionTest() throws SQLException {
try ( JdbcConnection polyphenyDbConnection = new JdbcConnection( false ) ) {
Connection connection = polyphenyDbConnection.getConnection();
try ( Statement statement = connection.createStatement() ) {
statement.executeUpdate( "CREATE TABLE transactions( "
+ "id INTEGER PRIMARY KEY, "
+ "ttext TEXT NOT NULL) " );

try {
PreparedStatement preparedInsert = connection.prepareStatement( "INSERT INTO transactions(id, ttext) VALUES (?, ?)" );

preparedInsert.setInt( 1, 1 );
preparedInsert.setString( 2, "Row A" );
preparedInsert.addBatch();

preparedInsert.setInt( 1, 2 );
preparedInsert.setString( 2, "Row 2" );
preparedInsert.addBatch();

preparedInsert.executeBatch();

connection.commit();
} catch ( Throwable t ) {
statement.executeUpdate( "DROP TABLE transactions" );
throw t;
}
}
}

try ( JdbcConnection polyphenyDbConnection = new JdbcConnection( false ) ) {
Connection connection = polyphenyDbConnection.getConnection();
try ( Statement statement = connection.createStatement() ) {
try {
PreparedStatement preparedSelect = connection.prepareStatement( "SELECT * FROM transactions WHERE id = ?" );
preparedSelect.setInt( 1, 1 );
preparedSelect.execute();
preparedSelect.getResultSet().close();

PreparedStatement preparedUpdate = connection.prepareStatement( "UPDATE transactions SET ttext = ? WHERE id = ?" );
preparedUpdate.setString( 1, "Row 1" );
preparedUpdate.setInt( 2, 1 );
preparedUpdate.execute();

connection.commit();

PreparedStatement preparedSelect2 = connection.prepareStatement( "SELECT * FROM transactions WHERE id = ?" );
preparedSelect2.setInt( 1, 1 );
preparedSelect2.execute();
preparedSelect2.getResultSet().close();

preparedSelect.setInt( 1, 1 );
preparedSelect.execute();

connection.commit();
} finally {
statement.executeUpdate( "DROP TABLE transactions" );
}
}
}
}

}
29 changes: 29 additions & 0 deletions dbms/src/test/java/org/polypheny/db/sql/view/ComplexViewTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import java.math.BigDecimal;
import java.sql.Connection;
import java.sql.Date;
import java.sql.PreparedStatement;
import java.sql.SQLException;
import java.sql.Statement;
import lombok.extern.slf4j.Slf4j;
Expand Down Expand Up @@ -1498,6 +1499,34 @@ public void testQ14() throws SQLException {
}


@Test
public void testCast() throws SQLException {
try ( JdbcConnection polyphenyDbConnection = new JdbcConnection( true ) ) {
Connection connection = polyphenyDbConnection.getConnection();
try ( Statement statement = connection.createStatement() ) {
initTables( statement );

try {
String query = """
SELECT l_extendedprice
FROM
lineitem
WHERE
l_extendedprice >= cast(? as Integer)""";

PreparedStatement prepare = connection.prepareStatement( query );
prepare.setString( 1, "3" );
prepare.execute();
connection.commit();
} finally {
connection.rollback();
dropTables( statement );
}
}
}
}


@Test
public void testQ15() throws SQLException {
try ( JdbcConnection polyphenyDbConnection = new JdbcConnection( true ) ) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -277,6 +277,8 @@ public static CottontailGrpc.Literal toData( PolyValue value, PolyType actualTyp
case DECIMAL: {
if ( value.isNumber() ) {
return builder.setStringData( value.asNumber().BigDecimalValue().toString() ).build();
} else if ( value.isString() ) {
return builder.setStringData( value.asString().value ).build();
}
break;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -411,7 +411,7 @@ private Enumerator<PolyValue[]> enumeratorBasedOnStatement() {
int updateCount = statement.getUpdateCount();
return Linq4j.singletonEnumerator( new PolyValue[]{ PolyLong.of( updateCount ) } );
}
} catch ( SQLException e ) {
} catch ( Throwable e ) {
throw Static.RESOURCE.exceptionWhilePerformingQueryOnJdbcSubSchema( sql ).ex( e );
} finally {
closeIfPossible( statement );
Expand Down Expand Up @@ -439,7 +439,7 @@ private Enumerator<PolyValue[]> enumeratorBasedOnPreparedStatement() {
return Linq4j.singletonEnumerator( new PolyValue[]{ PolyLong.of( updateCount ) } );
}
}
} catch ( SQLException e ) {
} catch ( Throwable e ) {
throw Static.RESOURCE.exceptionWhilePerformingQueryOnJdbcSubSchema( sql ).ex( e );
} finally {
closeIfPossible( preparedStatement );
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -466,7 +466,7 @@ public void commit( PolyXid xid ) {
if ( connectionFactory.hasConnectionHandler( xid ) ) {
try {
connectionFactory.getConnectionHandler( xid ).commit();
} catch ( ConnectionHandlerException e ) {
} catch ( Throwable e ) {
throw new GenericRuntimeException( e );
}
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ public class PIClient {
@Getter
private final String clientUUID;
private final LogicalUser catalogUser;
@Getter
@Setter
private Transaction currentTransaction;
private final TransactionManager transactionManager;
@Getter
Expand Down Expand Up @@ -94,7 +96,7 @@ private void commitCurrentTransactionUnsynchronized() throws PIServiceException
}
try {
currentTransaction.commit();
} catch ( TransactionException e ) {
} catch ( Throwable e ) {
throw new PIServiceException( "Committing current transaction failed: " + e.getMessage() );
} finally {
clearCurrentTransaction();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import java.util.List;
import java.util.Map;
import org.polypheny.db.algebra.type.AlgDataType;
import org.polypheny.db.algebra.type.AlgDataTypeField;
import org.polypheny.db.catalog.exceptions.GenericRuntimeException;
import org.polypheny.db.catalog.logistic.DataModel;
import org.polypheny.db.languages.LanguageManager;
Expand Down Expand Up @@ -143,6 +144,7 @@ public static void prepare( PIPreparedStatement piStatement ) {
Pair<Node, AlgDataType> validated = queryProcessor.validate( transaction, parsed, false );
AlgDataType parameterRowType = queryProcessor.getParameterRowType( validated.left );
piStatement.setParameterMetas( RelationalMetaRetriever.retrieveParameterMetas( parameterRowType ) );
piStatement.setParameterPolyTypes( parameterRowType.getFields().stream().map( AlgDataTypeField::getType ).toList() );
}

}
Loading

0 comments on commit 5da9c7e

Please sign in to comment.