From 2bde60b6a82dc39d08090bcb1aa92a48173e76e6 Mon Sep 17 00:00:00 2001 From: Tobias Hafner Date: Sat, 7 Dec 2024 16:17:19 +0100 Subject: [PATCH] Fix relational inserts without column names --- .../org/polypheny/db/adapter/Scannable.java | 2 +- .../catalogs/LogicalRelationalCatalog.java | 15 ++ .../polypheny/db/catalog/entity/Entity.java | 9 + .../catalog/entity/logical/LogicalColumn.java | 6 + .../catalog/entity/logical/LogicalTable.java | 32 +++- .../impl/logical/RelationalCatalog.java | 12 +- .../java/org/polypheny/db/rex/RexLiteral.java | 6 +- .../transaction/locking/IdentifierAdder.java | 140 +++++--------- .../transaction/locking/IdentifierUtils.java | 12 +- .../org/polypheny/db/ddl/DdlManagerImpl.java | 10 +- .../db/transaction/EntityIdentifierTests.java | 179 +++++++++++------- .../org/polypheny/db/sql/SqlProcessor.java | 6 +- .../language/validate/SqlValidatorImpl.java | 6 +- .../db/sql/sql2alg/SqlToAlgConverter.java | 2 +- 14 files changed, 249 insertions(+), 188 deletions(-) diff --git a/core/src/main/java/org/polypheny/db/adapter/Scannable.java b/core/src/main/java/org/polypheny/db/adapter/Scannable.java index 1d775179f9..b13ed8ea86 100644 --- a/core/src/main/java/org/polypheny/db/adapter/Scannable.java +++ b/core/src/main/java/org/polypheny/db/adapter/Scannable.java @@ -60,7 +60,7 @@ static PhysicalEntity createSubstitutionEntity( Scannable scannable, Context con int i = 0; for ( ColumnContext col : columnsInformations ) { - LogicalColumn column = new LogicalColumn( builder.getNewFieldId(), col.name, table.id, table.namespaceId, i, col.type, null, col.precision, null, null, null, col.nullable, Collation.getDefaultCollation(), null ); + LogicalColumn column = new LogicalColumn( builder.getNewFieldId(), false, col.name, table.id, table.namespaceId, i, col.type, null, col.precision, null, null, null, col.nullable, Collation.getDefaultCollation(), null ); columns.add( column ); i++; } diff --git a/core/src/main/java/org/polypheny/db/catalog/catalogs/LogicalRelationalCatalog.java b/core/src/main/java/org/polypheny/db/catalog/catalogs/LogicalRelationalCatalog.java index d9e35236a6..82e32a4ae8 100644 --- a/core/src/main/java/org/polypheny/db/catalog/catalogs/LogicalRelationalCatalog.java +++ b/core/src/main/java/org/polypheny/db/catalog/catalogs/LogicalRelationalCatalog.java @@ -123,6 +123,21 @@ public interface LogicalRelationalCatalog extends LogicalCatalog { */ LogicalColumn addColumn( String name, long tableId, int position, PolyType type, PolyType collectionsType, Integer length, Integer scale, Integer dimension, Integer cardinality, boolean nullable, Collation collation ); + /** + * Adds a column. + * + * @param name The name of the column + * @param tableId The id of the corresponding table + * @param position The ordinal position of the column (starting with 1) + * @param type The type of the column + * @param length The length of the field (if applicable, else null) + * @param scale The number of digits after the decimal point (if applicable, else null) + * @param nullable Weather the column can contain null values + * @param collation The collation of the field (if applicable, else null) + * @param isIdentifier Specifies weather the colum contains identifiers used for MVCC + * @return The id of the inserted column + */ + LogicalColumn addColumn( String name, long tableId, int position, PolyType type, PolyType collectionsType, Integer length, Integer scale, Integer dimension, Integer cardinality, boolean nullable, Collation collation, boolean isIdentifier ); /** * Renames a column diff --git a/core/src/main/java/org/polypheny/db/catalog/entity/Entity.java b/core/src/main/java/org/polypheny/db/catalog/entity/Entity.java index cf7bcb173c..8235d96ae8 100644 --- a/core/src/main/java/org/polypheny/db/catalog/entity/Entity.java +++ b/core/src/main/java/org/polypheny/db/catalog/entity/Entity.java @@ -97,6 +97,15 @@ public AlgDataType getTupleType() { }; } + public AlgDataType getTupleType(boolean hideIdentifier) { + return switch ( dataModel ) { + case RELATIONAL -> throw new UnsupportedOperationException( "Should be overwritten by child" ); + //TODO TH: adjust where necessary + case DOCUMENT -> DocumentType.ofId(); + case GRAPH -> GraphType.of(); + }; + } + @Override public AlgDataType getTupleType( AlgDataTypeFactory typeFactory ) { diff --git a/core/src/main/java/org/polypheny/db/catalog/entity/logical/LogicalColumn.java b/core/src/main/java/org/polypheny/db/catalog/entity/logical/LogicalColumn.java index 18b75cc056..25b1c2c460 100644 --- a/core/src/main/java/org/polypheny/db/catalog/entity/logical/LogicalColumn.java +++ b/core/src/main/java/org/polypheny/db/catalog/entity/logical/LogicalColumn.java @@ -47,6 +47,10 @@ public class LogicalColumn implements PolyObject, Comparable { @JsonProperty public long id; + @Serialize + @JsonProperty + public boolean isIdentifier; + @Serialize @JsonProperty public String name; @@ -105,6 +109,7 @@ public class LogicalColumn implements PolyObject, Comparable { public LogicalColumn( @Deserialize("id") final long id, + @Deserialize("isIdentifier") final boolean isIdentifier, @Deserialize("name") @NonNull final String name, @Deserialize("tableId") final long tableId, @Deserialize("namespaceId") final long namespaceId, @@ -119,6 +124,7 @@ public LogicalColumn( @Deserialize("collation") final Collation collation, @Deserialize("defaultValue") final LogicalDefaultValue defaultValue ) { this.id = id; + this.isIdentifier = isIdentifier; this.name = name; this.tableId = tableId; this.namespaceId = namespaceId; diff --git a/core/src/main/java/org/polypheny/db/catalog/entity/logical/LogicalTable.java b/core/src/main/java/org/polypheny/db/catalog/entity/logical/LogicalTable.java index 6f892d0e17..26b6d5d005 100644 --- a/core/src/main/java/org/polypheny/db/catalog/entity/logical/LogicalTable.java +++ b/core/src/main/java/org/polypheny/db/catalog/entity/logical/LogicalTable.java @@ -23,7 +23,9 @@ import io.activej.serializer.annotations.SerializeNullable; import java.io.Serial; import java.util.Comparator; +import java.util.LinkedList; import java.util.List; +import java.util.stream.Collectors; import lombok.EqualsAndHashCode; import lombok.NonNull; import lombok.RequiredArgsConstructor; @@ -40,7 +42,7 @@ import org.polypheny.db.catalog.logistic.DataModel; import org.polypheny.db.catalog.logistic.EntityType; import org.polypheny.db.schema.ColumnStrategy; -import org.polypheny.db.transaction.locking.Lockable; +import org.polypheny.db.transaction.locking.IdentifierUtils; @EqualsAndHashCode(callSuper = false) @SuperBuilder(toBuilder = true) @@ -75,9 +77,15 @@ public LogicalTable( @Override public AlgDataType getTupleType() { + return getTupleType( false ); + } + + + @Override + public AlgDataType getTupleType( boolean hideIdentifier ) { final AlgDataTypeFactory.Builder fieldInfo = AlgDataTypeFactory.DEFAULT.builder(); - for ( LogicalColumn column : Catalog.snapshot().rel().getColumns( id ).stream().sorted( Comparator.comparingInt( a -> a.position ) ).toList() ) { + for ( LogicalColumn column : getColumns(hideIdentifier).stream().sorted( Comparator.comparingInt( a -> a.position ) ).toList() ) { AlgDataType sqlType = column.getAlgDataType( AlgDataTypeFactory.DEFAULT ); fieldInfo.add( column.id, column.name, null, sqlType ).nullable( column.nullable ); } @@ -93,11 +101,23 @@ public Expression asExpression() { public List getColumnStrategies() { - return getColumns().stream().map( c -> c.nullable ? ColumnStrategy.NULLABLE : ColumnStrategy.NOT_NULLABLE ).toList(); + return getColumnStrategies(false); + } + + public List getColumnStrategies(boolean hideIdentifiers) { + return getColumns(hideIdentifiers).stream().map( c -> c.nullable ? ColumnStrategy.NULLABLE : ColumnStrategy.NOT_NULLABLE ).toList(); } public List getColumns() { + return getColumns( false ); + } + + + public List getColumns( boolean hideIdentifier ) { + if ( hideIdentifier ) { + return Catalog.snapshot().rel().getColumns( id ).stream().filter( c -> !c.isIdentifier() ).toList(); + } return Catalog.snapshot().rel().getColumns( id ); } @@ -112,6 +132,11 @@ public List getColumnNames() { } + public List getColumnNames(boolean hideIdentifier) { + return getColumns(hideIdentifier).stream().map( c -> c.name ).toList(); + } + + @Override public String getNamespaceName() { return Catalog.snapshot().getNamespace( namespaceId ).orElseThrow().name; @@ -159,5 +184,4 @@ public String toString() { } - } diff --git a/core/src/main/java/org/polypheny/db/catalog/impl/logical/RelationalCatalog.java b/core/src/main/java/org/polypheny/db/catalog/impl/logical/RelationalCatalog.java index 568eedd67e..d4ebd1842d 100644 --- a/core/src/main/java/org/polypheny/db/catalog/impl/logical/RelationalCatalog.java +++ b/core/src/main/java/org/polypheny/db/catalog/impl/logical/RelationalCatalog.java @@ -290,11 +290,19 @@ public void deleteKey( long id ) { change( CatalogEvent.KEY_DROPPED, id, null ); } + @Override + public LogicalColumn addColumn( String name, long tableId, int position, PolyType type, PolyType collectionsType, Integer length, Integer scale, Integer dimension, Integer cardinality, boolean nullable, Collation collation) { + long id = idBuilder.getNewFieldId(); + LogicalColumn column = new LogicalColumn( id, false, name, tableId, logicalNamespace.id, position, type, collectionsType, length, scale, dimension, cardinality, nullable, collation, null ); + columns.put( id, column ); + change( CatalogEvent.LOGICAL_REL_FIELD_CREATED, null, id ); + return column; + } @Override - public LogicalColumn addColumn( String name, long tableId, int position, PolyType type, PolyType collectionsType, Integer length, Integer scale, Integer dimension, Integer cardinality, boolean nullable, Collation collation ) { + public LogicalColumn addColumn( String name, long tableId, int position, PolyType type, PolyType collectionsType, Integer length, Integer scale, Integer dimension, Integer cardinality, boolean nullable, Collation collation, boolean isIdentifier ) { long id = idBuilder.getNewFieldId(); - LogicalColumn column = new LogicalColumn( id, name, tableId, logicalNamespace.id, position, type, collectionsType, length, scale, dimension, cardinality, nullable, collation, null ); + LogicalColumn column = new LogicalColumn( id, isIdentifier, name, tableId, logicalNamespace.id, position, type, collectionsType, length, scale, dimension, cardinality, nullable, collation, null ); columns.put( id, column ); change( CatalogEvent.LOGICAL_REL_FIELD_CREATED, null, id ); return column; diff --git a/core/src/main/java/org/polypheny/db/rex/RexLiteral.java b/core/src/main/java/org/polypheny/db/rex/RexLiteral.java index abd8d2318f..8b2cccdad2 100644 --- a/core/src/main/java/org/polypheny/db/rex/RexLiteral.java +++ b/core/src/main/java/org/polypheny/db/rex/RexLiteral.java @@ -441,11 +441,7 @@ private static void printAsJava( PolyValue value, PrintWriter pw, PolyType typeN pw.print( value.asBoolean().value ); break; case DECIMAL: - assert value.isBigDecimal() || value.isLong(); - if (value.isLong()) { - pw.print( value.asLong().value ); - break; - } + assert value.isBigDecimal(); pw.print( value.asBigDecimal().value ); break; case DOUBLE: diff --git a/core/src/main/java/org/polypheny/db/transaction/locking/IdentifierAdder.java b/core/src/main/java/org/polypheny/db/transaction/locking/IdentifierAdder.java index ca1b330f13..db29f0b2cf 100644 --- a/core/src/main/java/org/polypheny/db/transaction/locking/IdentifierAdder.java +++ b/core/src/main/java/org/polypheny/db/transaction/locking/IdentifierAdder.java @@ -17,6 +17,7 @@ package org.polypheny.db.transaction.locking; import com.google.common.collect.ImmutableList; +import java.util.ArrayList; import java.util.LinkedList; import java.util.List; import java.util.Optional; @@ -58,19 +59,14 @@ import org.polypheny.db.algebra.logical.relational.LogicalRelTableFunctionScan; import org.polypheny.db.algebra.logical.relational.LogicalRelUnion; import org.polypheny.db.algebra.logical.relational.LogicalRelValues; -import org.polypheny.db.algebra.type.AlgDataTypeFactoryImpl; +import org.polypheny.db.algebra.type.AlgDataType; import org.polypheny.db.algebra.type.AlgDataTypeField; import org.polypheny.db.rex.RexIndexRef; import org.polypheny.db.rex.RexLiteral; import org.polypheny.db.rex.RexNode; -import org.polypheny.db.type.PolyType; -import org.polypheny.db.type.entity.PolyValue; -import org.polypheny.db.type.entity.numerical.PolyLong; public class IdentifierAdder extends AlgShuttleImpl { - private static final PolyLong missingIdentifier = new PolyLong( IdentifierUtils.MISSING_IDENTIFIER ); - public static AlgRoot process( AlgRoot root ) { return root.withAlg( root.alg.accept( new IdentifierAdder() ) ); @@ -103,27 +99,7 @@ public AlgNode visit( LogicalRelTableFunctionScan scan ) { @Override public AlgNode visit( LogicalRelValues values ) { - ImmutableList> immutableValues = values.tuples.stream() - .map( row -> { - RexLiteral identifierLiteral = row.get( 0 ); - if ( !identifierLiteral.getValue().equals( missingIdentifier ) ) { - /* - This method is only called on the top level of an insert. - The values present are thus all inserted by the user. - If identifiers are present at this stage, they were added by the user. - */ - IdentifierUtils.throwIllegalFieldName(); - } - PolyValue identifier = IdentifierUtils.getIdentifier(); - return ImmutableList.builder() - .add( new RexLiteral( identifier, identifierLiteral.getType(), PolyType.DECIMAL ) ) - .addAll( row.subList( 1, row.size() ) ) - .build(); - - } ) - .collect( ImmutableList.toImmutableList() ); - - return LogicalRelValues.create( values.getCluster(), values.getRowType(), immutableValues ); + return values; } @@ -134,87 +110,68 @@ public AlgNode visit( LogicalRelFilter filter ) { @Override - public AlgNode visit(LogicalRelProject project) { + public AlgNode visit( LogicalRelProject project ) { + /* + Projects at this stage never contain references to values for the identifier column. + New identifiers must thus be drawn and the projection adjusted accordingly. + */ + // Detect underpopulated columns if the user specifies fewer values than there are columns in the table - Optional indexOpt = findIdentifierIndex(project); - if (indexOpt.isEmpty()) { + Optional identifier = findIdentifierIndex( project ); + if ( identifier.isEmpty() ) { return project; } - - int index = indexOpt.get(); + int identifierIndex = identifier.get(); AlgNode input = project.getInput(); - if (!(input instanceof LogicalRelValues inputValues)) { - return project; - } - - if ( hasLessInputsThanColumns(project, inputValues)) { - return createNewProjectWithIdentifiers(project, inputValues, index); + if ( input instanceof LogicalRelValues inputValues ) { + return createNewProjectWithIdentifiers( project, inputValues, identifierIndex ); } - return visitChild(project, 0, project.getInput()); } - private Optional findIdentifierIndex(LogicalRelProject project) { + + private Optional findIdentifierIndex( LogicalRelProject project ) { return project.getRowType().getFields().stream() - .filter(field -> field.getName().equals(IdentifierUtils.IDENTIFIER_KEY)) - .map(AlgDataTypeField::getIndex) + .filter( field -> field.getName().equals( IdentifierUtils.IDENTIFIER_KEY ) ) + .map( AlgDataTypeField::getIndex ) .findFirst(); } - private boolean hasLessInputsThanColumns(LogicalRelProject project, LogicalRelValues inputValues) { - long fieldCount = project.getRowType().getFieldCount(); - long inputFieldCount = inputValues.tuples.get(0).size(); - return inputFieldCount < fieldCount; - } - private AlgNode createNewProjectWithIdentifiers(LogicalRelProject project, LogicalRelValues inputValues, int index) { - ImmutableList> immutableValues = adjustInputValues(inputValues, index); + private AlgNode createNewProjectWithIdentifiers( LogicalRelProject project, LogicalRelValues inputValues, int identifierIndex ) { + ImmutableList> newValues = adjustInputValues( inputValues, identifierIndex ); LogicalRelValues newInput = new LogicalRelValues( inputValues.getCluster(), inputValues.getTraitSet(), - inputValues.getRowType(), - immutableValues + project.getRowType(), + newValues ); - - List newProjects = createNewProjects(project, inputValues); - return project.copy(project.getTraitSet(), newInput, newProjects, project.getRowType()); + List newProjects = createNewProjects( project, newInput ); + return project.copy( project.getTraitSet(), newInput, newProjects, project.getRowType() ); } - private ImmutableList> adjustInputValues(LogicalRelValues inputValues, int index) { + + private ImmutableList> adjustInputValues( LogicalRelValues inputValues, int identifierIndex ) { return inputValues.tuples.stream() - .map(row -> { - PolyValue identifier = IdentifierUtils.getIdentifier(); - ImmutableList.Builder builder = ImmutableList.builder(); - builder.addAll(row.subList(0, index)); - builder.add(new RexLiteral( - identifier, - AlgDataTypeFactoryImpl.DEFAULT.createPolyType(identifier.getType()), - PolyType.DECIMAL - )); - builder.addAll(row.subList(index, row.size())); - return builder.build(); - }) - .collect(ImmutableList.toImmutableList()); + .map( row -> ImmutableList.builder() + .addAll( row.subList( 0, identifierIndex ) ) + .add( IdentifierUtils.getIdentifierAsLiteral() ) + .addAll( row.subList( identifierIndex, row.size() ) ) + .build() ) + .collect( ImmutableList.toImmutableList() ); } + private List createNewProjects(LogicalRelProject project, LogicalRelValues inputValues) { - List newProjects = new LinkedList<>(); - long fieldCount = project.getRowType().getFieldCount(); - long inputFieldCount = inputValues.tuples.get(0).size(); - - for (int position = 0; position < fieldCount; position++) { - if (position < inputFieldCount) { - newProjects.add(new RexIndexRef( - position, - project.getRowType().getFields().get(position).getType() - )); - } else { - newProjects.add(new RexLiteral( - null, - project.getRowType().getFields().get(position).getType(), - project.getRowType().getFields().get(position).getType().getPolyType() - )); - } + List newProjects = new ArrayList<>(); + List fields = project.getRowType().getFields(); + int inputFieldCount = inputValues.tuples.get(0).size(); + + for (int position = 0; position < fields.size(); position++) { + AlgDataType fieldType = fields.get(position).getType(); + newProjects.add(position < inputFieldCount + ? new RexIndexRef(position, fieldType) + : new RexLiteral(null, fieldType, fieldType.getPolyType())); } return newProjects; @@ -271,18 +228,7 @@ public AlgNode visit( LogicalConditionalExecute lce ) { @Override public AlgNode visit( LogicalRelModify modify ) { - switch ( modify.getOperation() ) { - case UPDATE -> { - if ( modify.getUpdateColumns().contains( IdentifierUtils.IDENTIFIER_KEY ) ) { - IdentifierUtils.throwIllegalFieldName(); - } - return modify; - } - case INSERT -> { - return visitChildren( modify ); - } - } - return modify; + return visitChildren( modify ); } diff --git a/core/src/main/java/org/polypheny/db/transaction/locking/IdentifierUtils.java b/core/src/main/java/org/polypheny/db/transaction/locking/IdentifierUtils.java index d334f97381..eff4eb07c9 100644 --- a/core/src/main/java/org/polypheny/db/transaction/locking/IdentifierUtils.java +++ b/core/src/main/java/org/polypheny/db/transaction/locking/IdentifierUtils.java @@ -16,13 +16,18 @@ package org.polypheny.db.transaction.locking; +import java.math.BigDecimal; import java.text.MessageFormat; import java.util.LinkedList; import java.util.List; import java.util.stream.Collectors; +import org.polypheny.db.algebra.type.AlgDataType; +import org.polypheny.db.algebra.type.AlgDataTypeFactoryImpl; import org.polypheny.db.catalog.logistic.Collation; import org.polypheny.db.ddl.DdlManager.ColumnTypeInformation; import org.polypheny.db.ddl.DdlManager.FieldInformation; +import org.polypheny.db.rex.RexBuilder; +import org.polypheny.db.rex.RexLiteral; import org.polypheny.db.type.PolyType; import org.polypheny.db.type.entity.numerical.PolyLong; @@ -30,6 +35,7 @@ public class IdentifierUtils { public static final String IDENTIFIER_KEY = "_eid"; public static final long MISSING_IDENTIFIER = 0; + public static final AlgDataType IDENTIFIER_ALG_TYPE = AlgDataTypeFactoryImpl.DEFAULT.createPolyType( PolyType.BIGINT ); public static final ColumnTypeInformation IDENTIFIER_COLUMN_TYPE = new ColumnTypeInformation( PolyType.BIGINT, // binary not supported by hsqldb TODO TH: check for other stores, datatypes @@ -49,9 +55,11 @@ public class IdentifierUtils { 1 ); + private static final RexBuilder REX_BUILDER = new RexBuilder( AlgDataTypeFactoryImpl.DEFAULT ); - public static PolyLong getIdentifier() { - return new PolyLong( IdentifierRegistry.INSTANCE.getEntryIdentifier() ); + + public static RexLiteral getIdentifierAsLiteral() { + return REX_BUILDER.makeExactLiteral( BigDecimal.valueOf( IdentifierRegistry.INSTANCE.getEntryIdentifier() ) ); } diff --git a/dbms/src/main/java/org/polypheny/db/ddl/DdlManagerImpl.java b/dbms/src/main/java/org/polypheny/db/ddl/DdlManagerImpl.java index 9cb37d1a35..73d77ececc 100644 --- a/dbms/src/main/java/org/polypheny/db/ddl/DdlManagerImpl.java +++ b/dbms/src/main/java/org/polypheny/db/ddl/DdlManagerImpl.java @@ -1758,7 +1758,7 @@ public void createMaterializedView( String viewName, long namespaceId, AlgRoot a Map ids = new LinkedHashMap<>(); for ( FieldInformation field : fields ) { - ids.put( field.name(), addColumn( namespaceId, field.name(), field.typeInformation(), field.collation(), field.defaultValue(), view.id, field.position() ) ); + ids.put( field.name(), addColumn( namespaceId, field.name(), field.typeInformation(), field.collation(), field.defaultValue(), view.id, field.position(), false ) ); } // Sets previously created primary key @@ -2069,7 +2069,8 @@ public void createTable( long namespaceId, String name, List f fields = IdentifierUtils.addIdentifierFieldIfAbsent( fields ); for ( FieldInformation information : fields ) { - ids.put( information.name(), addColumn( namespaceId, information.name(), information.typeInformation(), information.collation(), information.defaultValue(), logical.id, information.position() + 1 ) ); // pos + 1 to make space for entry identifier column + ids.put( information.name(), addColumn( namespaceId, information.name(), information.typeInformation(), information.collation(), + information.defaultValue(), logical.id, information.position() + 1, information.name().equals( IdentifierUtils.IDENTIFIER_KEY ) ) ); // pos + 1 to make space for entry identifier column } List pkIds = new ArrayList<>(); @@ -2757,7 +2758,7 @@ public void dropTablePartition( LogicalTable table, Statement statement ) throws } - private LogicalColumn addColumn( long namespaceId, String columnName, ColumnTypeInformation typeInformation, Collation collation, PolyValue defaultValue, long tableId, int position ) { + private LogicalColumn addColumn( long namespaceId, String columnName, ColumnTypeInformation typeInformation, Collation collation, PolyValue defaultValue, long tableId, int position, boolean isIdentifier ) { columnName = adjustNameIfNeeded( columnName, namespaceId ); // check arrays to be correctly typed checkValidType( typeInformation ); @@ -2773,7 +2774,8 @@ private LogicalColumn addColumn( long namespaceId, String columnName, ColumnType typeInformation.dimension(), typeInformation.cardinality(), typeInformation.nullable(), - collation + collation, + isIdentifier ); // Add default value diff --git a/dbms/src/test/java/org/polypheny/db/transaction/EntityIdentifierTests.java b/dbms/src/test/java/org/polypheny/db/transaction/EntityIdentifierTests.java index 4d20e6d646..578b953a72 100644 --- a/dbms/src/test/java/org/polypheny/db/transaction/EntityIdentifierTests.java +++ b/dbms/src/test/java/org/polypheny/db/transaction/EntityIdentifierTests.java @@ -60,15 +60,61 @@ public void testCreateTable() throws SQLException { } } + + /* + Modify <- Project <- RelValues ('first', 'second') + */ @Test - public void testCreateTableIllegalColumnName() throws SQLException { + public void testInsertUnparameterizedWithColumnNames() throws SQLException { + try ( JdbcConnection jdbcConnection = new JdbcConnection( true ) ) { + Connection connection = jdbcConnection.getConnection(); + try ( Statement statement = connection.createStatement() ) { + try { + statement.executeUpdate( "CREATE TABLE identifiers (a VARCHAR(8) NOT NULL, b VARCHAR(8), PRIMARY KEY (a))" ); + statement.executeUpdate( "INSERT INTO identifiers (a, b) VALUES ('first', 'second')" ); + + connection.commit(); + } finally { + statement.executeUpdate( "DROP TABLE IF EXISTS identifiers" ); + connection.commit(); + } + } + } + } + + + /* + Modify <- Project <- RelValues ('first', 'second') + _eid is listed as varchar in rowtype + */ + @Test + public void testInsertUnparameterizedNoColumnNames() throws SQLException { try ( JdbcConnection jdbcConnection = new JdbcConnection( true ) ) { Connection connection = jdbcConnection.getConnection(); try ( Statement statement = connection.createStatement() ) { try { + statement.executeUpdate( "CREATE TABLE identifiers (a VARCHAR(8) NOT NULL, b VARCHAR(8), PRIMARY KEY (a))" ); + statement.executeUpdate( "INSERT INTO identifiers VALUES ('first', 'second')" ); + connection.commit(); + } finally { + statement.executeUpdate( "DROP TABLE IF EXISTS identifiers" ); + connection.commit(); + } + } + } + } + + + @Test + public void testInsertUnparameterizedIdentifierManipulation() throws SQLException { + try ( JdbcConnection jdbcConnection = new JdbcConnection( true ) ) { + Connection connection = jdbcConnection.getConnection(); + try ( Statement statement = connection.createStatement() ) { + try { + statement.executeUpdate( "CREATE TABLE identifiers (a INTEGER NOT NULL, b INTEGER, PRIMARY KEY (a))" ); assertThrows( PrismInterfaceServiceException.class, - () -> statement.executeUpdate( "CREATE TABLE identifiers (a INTEGER NOT NULL, _eid INTEGER, PRIMARY KEY (a))" ) + () -> statement.executeUpdate( "INSERT INTO identifiers VALUES (-32, 2, 3)" ) ); connection.commit(); } finally { @@ -80,18 +126,63 @@ public void testCreateTableIllegalColumnName() throws SQLException { } + /* + + + */ @Test - public void testInsertUnparameterizedWithColumnNames() throws SQLException { + public void testInsertFromTableWithColumnNames() throws SQLException { try ( JdbcConnection jdbcConnection = new JdbcConnection( true ) ) { Connection connection = jdbcConnection.getConnection(); try ( Statement statement = connection.createStatement() ) { try { - statement.executeUpdate( "CREATE TABLE identifiers (a VARCHAR(8) NOT NULL, b VARCHAR(8), PRIMARY KEY (a))" ); - statement.executeUpdate( "INSERT INTO identifiers (a, b) VALUES ('first', 'second')" ); + statement.executeUpdate( "CREATE TABLE identifiers1 (a VARCHAR(8) NOT NULL, b VARCHAR(8), PRIMARY KEY (a))" ); + statement.executeUpdate( "INSERT INTO identifiers1 (a, b) VALUES ('first', 'second'), ('third', 'fourth')" ); + + statement.executeUpdate( "CREATE TABLE identifiers2 (x VARCHAR(8) NOT NULL, y VARCHAR(8), PRIMARY KEY (x))" ); + statement.executeUpdate( "INSERT INTO identifiers2 (x, y) SELECT a, b FROM identifiers1" ); + + // check that new identifiers had been assigned instead of copying from the first table + try ( ResultSet rs = statement.executeQuery( """ + SELECT 1 + FROM identifiers1 id1 + WHERE EXISTS ( + SELECT 1 + FROM identifiers2 id2 + WHERE id1._eid = id2._eid + ); + """ ) ) { + TestHelper.checkResultSet( rs, List.of() ); + } connection.commit(); } finally { - statement.executeUpdate( "DROP TABLE IF EXISTS identifiers" ); + statement.executeUpdate( "DROP TABLE IF EXISTS identifiers1" ); + statement.executeUpdate( "DROP TABLE IF EXISTS identifiers2" ); + connection.commit(); + } + } + } + } + + + @Test + public void testInsertFromTableWithoutTargetColumnNames() throws SQLException { + try ( JdbcConnection jdbcConnection = new JdbcConnection( true ) ) { + Connection connection = jdbcConnection.getConnection(); + try ( Statement statement = connection.createStatement() ) { + try { + statement.executeUpdate( "CREATE TABLE identifiers1 (a VARCHAR(8) NOT NULL, b VARCHAR(8), PRIMARY KEY (a))" ); + statement.executeUpdate( "INSERT INTO identifiers1 (a, b) VALUES ('first', 'second'), ('third', 'fourth')" ); + + statement.executeUpdate( "CREATE TABLE identifiers2 (x VARCHAR(8) NOT NULL, y VARCHAR(8), PRIMARY KEY (x))" ); + statement.executeUpdate( "INSERT INTO identifiers2 SELECT a, b FROM identifiers1" ); + + //TODO TH: check that new identifiers had been assigned instead of copying from the first table + connection.commit(); + } finally { + statement.executeUpdate( "DROP TABLE IF EXISTS identifiers1" ); + statement.executeUpdate( "DROP TABLE IF EXISTS identifiers2" ); connection.commit(); } } @@ -200,30 +291,6 @@ WHERE EXISTS ( } - @Test - public void testInsertFromTableWithoutTargetColumnNames() throws SQLException { - try ( JdbcConnection jdbcConnection = new JdbcConnection( true ) ) { - Connection connection = jdbcConnection.getConnection(); - try ( Statement statement = connection.createStatement() ) { - try { - statement.executeUpdate( "CREATE TABLE identifiers1 (a VARCHAR(8) NOT NULL, b VARCHAR(8), PRIMARY KEY (a))" ); - statement.executeUpdate( "INSERT INTO identifiers1 (a, b) VALUES ('first', 'second'), ('third', 'fourth')" ); - - statement.executeUpdate( "CREATE TABLE identifiers2 (x VARCHAR(8) NOT NULL, y VARCHAR(8), PRIMARY KEY (x))" ); - statement.executeUpdate( "INSERT INTO identifiers2 SELECT a, b FROM identifiers1" ); - - //TODO TH: check that new identifiers had been assigned instead of copying from the first table - connection.commit(); - } finally { - statement.executeUpdate( "DROP TABLE IF EXISTS identifiers1" ); - statement.executeUpdate( "DROP TABLE IF EXISTS identifiers2" ); - connection.commit(); - } - } - } - } - - @Test public void testInsertFromTableWithoutColumnNames() throws SQLException { try ( JdbcConnection jdbcConnection = new JdbcConnection( true ) ) { @@ -381,25 +448,6 @@ public void testInsertMultipleParameterizedDefaultOmitted() throws SQLException } - @Test - // TODO TH: Does this work? - public void testInsertUnparameterizedNoColumnNames() throws SQLException { - try ( JdbcConnection jdbcConnection = new JdbcConnection( true ) ) { - Connection connection = jdbcConnection.getConnection(); - try ( Statement statement = connection.createStatement() ) { - try { - statement.executeUpdate( "CREATE TABLE identifiers (a VARCHAR(8) NOT NULL, b VARCHAR(8), PRIMARY KEY (a))" ); - statement.executeUpdate( "INSERT INTO identifiers VALUES ('first', 'second')" ); - connection.commit(); - } finally { - statement.executeUpdate( "DROP TABLE IF EXISTS identifiers" ); - connection.commit(); - } - } - } - } - - @Test // TODO TH: Does this work? public void testInsertPreparedNoColumnNames() throws SQLException { @@ -465,15 +513,16 @@ public void testInsertUnparameterizedColumnNameConflictDifferentType() throws SQ @Test - public void testInsertUnparameterizedIdentifierManipulationInsert() throws SQLException { + public void testUpdateUnparameterizedIdentifierManipulation() throws SQLException { try ( JdbcConnection jdbcConnection = new JdbcConnection( true ) ) { Connection connection = jdbcConnection.getConnection(); try ( Statement statement = connection.createStatement() ) { try { statement.executeUpdate( "CREATE TABLE identifiers (a INTEGER NOT NULL, b INTEGER, PRIMARY KEY (a))" ); + statement.executeUpdate( "INSERT INTO identifiers (a, b) VALUES (1, 2)" ); assertThrows( PrismInterfaceServiceException.class, - () -> statement.executeUpdate( "INSERT INTO identifiers VALUES (-32, 2, 3)" ) + () -> statement.executeUpdate( "UPDATE identifiers SET _eid = 32 WHERE a = 1 AND b = 2" ) ); connection.commit(); } finally { @@ -486,16 +535,15 @@ public void testInsertUnparameterizedIdentifierManipulationInsert() throws SQLEx @Test - public void testUpdateUnparameterizedIdentifierManipulation() throws SQLException { + public void testDropIdentifierColumnUnparameterized() throws SQLException { try ( JdbcConnection jdbcConnection = new JdbcConnection( true ) ) { Connection connection = jdbcConnection.getConnection(); try ( Statement statement = connection.createStatement() ) { try { statement.executeUpdate( "CREATE TABLE identifiers (a INTEGER NOT NULL, b INTEGER, PRIMARY KEY (a))" ); - statement.executeUpdate( "INSERT INTO identifiers (a, b) VALUES (1, 2)" ); assertThrows( PrismInterfaceServiceException.class, - () -> statement.executeUpdate( "UPDATE identifiers SET _eid = 32 WHERE a = 1 AND b = 2" ) + () -> statement.executeUpdate( "ALTER TABLE identifiers DROP COLUMN _eid" ) ); connection.commit(); } finally { @@ -508,7 +556,7 @@ public void testUpdateUnparameterizedIdentifierManipulation() throws SQLExceptio @Test - public void testDropIdentifierColumnUnparameterized() throws SQLException { + public void testAddIdentifierColumnUnparameterized() throws SQLException { try ( JdbcConnection jdbcConnection = new JdbcConnection( true ) ) { Connection connection = jdbcConnection.getConnection(); try ( Statement statement = connection.createStatement() ) { @@ -516,8 +564,9 @@ public void testDropIdentifierColumnUnparameterized() throws SQLException { statement.executeUpdate( "CREATE TABLE identifiers (a INTEGER NOT NULL, b INTEGER, PRIMARY KEY (a))" ); assertThrows( PrismInterfaceServiceException.class, - () -> statement.executeUpdate( "ALTER TABLE identifiers DROP COLUMN _eid" ) + () -> statement.executeUpdate( "ALTER TABLE identifiers ADD COLUMN _eid BIGINT" ) ); + connection.commit(); } finally { statement.executeUpdate( "DROP TABLE IF EXISTS identifiers" ); @@ -529,7 +578,7 @@ public void testDropIdentifierColumnUnparameterized() throws SQLException { @Test - public void testAddIdentifierColumnUnparameterized() throws SQLException { + public void testRenameIdentifierColumnUnparameterized() throws SQLException { try ( JdbcConnection jdbcConnection = new JdbcConnection( true ) ) { Connection connection = jdbcConnection.getConnection(); try ( Statement statement = connection.createStatement() ) { @@ -537,9 +586,8 @@ public void testAddIdentifierColumnUnparameterized() throws SQLException { statement.executeUpdate( "CREATE TABLE identifiers (a INTEGER NOT NULL, b INTEGER, PRIMARY KEY (a))" ); assertThrows( PrismInterfaceServiceException.class, - () -> statement.executeUpdate( "ALTER TABLE identifiers ADD COLUMN _eid BIGINT" ) + () -> statement.executeUpdate( "ALTER TABLE identifiers RENAME COLUMN _eid TO nowItsBroken" ) ); - connection.commit(); } finally { statement.executeUpdate( "DROP TABLE IF EXISTS identifiers" ); @@ -551,7 +599,7 @@ public void testAddIdentifierColumnUnparameterized() throws SQLException { @Test - public void testRenameIdentifierColumnUnparameterized() throws SQLException { + public void testRenameNonIdentifierColumnUnparameterized() throws SQLException { try ( JdbcConnection jdbcConnection = new JdbcConnection( true ) ) { Connection connection = jdbcConnection.getConnection(); try ( Statement statement = connection.createStatement() ) { @@ -559,7 +607,7 @@ public void testRenameIdentifierColumnUnparameterized() throws SQLException { statement.executeUpdate( "CREATE TABLE identifiers (a INTEGER NOT NULL, b INTEGER, PRIMARY KEY (a))" ); assertThrows( PrismInterfaceServiceException.class, - () -> statement.executeUpdate( "ALTER TABLE identifiers RENAME COLUMN _eid TO nowItsBroken" ) + () -> statement.executeUpdate( "ALTER TABLE identifiers RENAME COLUMN b TO _eid" ) ); connection.commit(); } finally { @@ -572,7 +620,7 @@ public void testRenameIdentifierColumnUnparameterized() throws SQLException { @Test - public void testRenameNonIdentifierColumnUnparameterized() throws SQLException { + public void testChangeDataTypeOfIdentifierColumnUnparameterized() throws SQLException { try ( JdbcConnection jdbcConnection = new JdbcConnection( true ) ) { Connection connection = jdbcConnection.getConnection(); try ( Statement statement = connection.createStatement() ) { @@ -580,7 +628,7 @@ public void testRenameNonIdentifierColumnUnparameterized() throws SQLException { statement.executeUpdate( "CREATE TABLE identifiers (a INTEGER NOT NULL, b INTEGER, PRIMARY KEY (a))" ); assertThrows( PrismInterfaceServiceException.class, - () -> statement.executeUpdate( "ALTER TABLE identifiers RENAME COLUMN b TO _eid" ) + () -> statement.executeUpdate( "ALTER TABLE identifiers MODIFY COLUMN _eid SET TYPE VARCHAR(15)" ) ); connection.commit(); } finally { @@ -593,15 +641,14 @@ public void testRenameNonIdentifierColumnUnparameterized() throws SQLException { @Test - public void testChangeDataTypeOfIdentifierColumnUnparameterized() throws SQLException { + public void testCreateTableIllegalColumnName() throws SQLException { try ( JdbcConnection jdbcConnection = new JdbcConnection( true ) ) { Connection connection = jdbcConnection.getConnection(); try ( Statement statement = connection.createStatement() ) { try { - statement.executeUpdate( "CREATE TABLE identifiers (a INTEGER NOT NULL, b INTEGER, PRIMARY KEY (a))" ); assertThrows( PrismInterfaceServiceException.class, - () -> statement.executeUpdate( "ALTER TABLE identifiers MODIFY COLUMN _eid SET TYPE VARCHAR(15)" ) + () -> statement.executeUpdate( "CREATE TABLE identifiers (a INTEGER NOT NULL, _eid INTEGER, PRIMARY KEY (a))" ) ); connection.commit(); } finally { diff --git a/plugins/sql-language/src/main/java/org/polypheny/db/sql/SqlProcessor.java b/plugins/sql-language/src/main/java/org/polypheny/db/sql/SqlProcessor.java index f688807f82..bbf97f6883 100644 --- a/plugins/sql-language/src/main/java/org/polypheny/db/sql/SqlProcessor.java +++ b/plugins/sql-language/src/main/java/org/polypheny/db/sql/SqlProcessor.java @@ -302,7 +302,7 @@ private void processColumns( SqlNode[][] newValues, SqlInsert insert ) { int pos = 0; - for ( LogicalColumn column : catalogTable.getColumns() ) { + for ( LogicalColumn column : catalogTable.getColumns(true) ) { newColumnList.add( new SqlIdentifier( column.name, ParserPos.ZERO ) ); for ( int i = 0; i < ((SqlBasicCall) insert.getSource()).getOperands().length; i++ ) { @@ -325,9 +325,9 @@ private void processColumns( public static int computeTargetSize( LogicalTable catalogTable, SqlNodeList oldColumnList, DataModel dataModel ) { - int size = catalogTable.getColumns().size(); + int size = catalogTable.getColumns(true).size(); if ( dataModel == DataModel.DOCUMENT ) { - List columnNames = catalogTable.getColumnNames(); + List columnNames = catalogTable.getColumnNames(true); size += (int) oldColumnList.getSqlList() .stream() .filter( column -> !columnNames.contains( ((SqlIdentifier) column).names.get( 0 ) ) ) diff --git a/plugins/sql-language/src/main/java/org/polypheny/db/sql/language/validate/SqlValidatorImpl.java b/plugins/sql-language/src/main/java/org/polypheny/db/sql/language/validate/SqlValidatorImpl.java index 2546c34ddd..325105fecd 100644 --- a/plugins/sql-language/src/main/java/org/polypheny/db/sql/language/validate/SqlValidatorImpl.java +++ b/plugins/sql-language/src/main/java/org/polypheny/db/sql/language/validate/SqlValidatorImpl.java @@ -3905,7 +3905,7 @@ protected AlgDataType createTargetRowType( Entity table, SqlNodeList targetColum * @return Rowtype */ protected AlgDataType createTargetRowType( Entity table, SqlNodeList targetColumnList, boolean append, boolean allowDynamic ) { - AlgDataType baseRowType = table.getTupleType(); + AlgDataType baseRowType = table.getTupleType(true); // if ( targetColumnList == null ) { return baseRowType; } @@ -4000,8 +4000,8 @@ private void checkFieldCount( SqlNode node, Entity table, SqlNode source, AlgDat throw newValidationError( node, RESOURCE.unmatchInsertColumn( targetFieldCount, sourceFieldCount ) ); } // Ensure that non-nullable fields are targeted. - final List strategies = table.unwrap( LogicalTable.class ).orElseThrow().getColumnStrategies(); - for ( final AlgDataTypeField field : table.getTupleType().getFields() ) { + final List strategies = table.unwrap( LogicalTable.class ).orElseThrow().getColumnStrategies(true); + for ( final AlgDataTypeField field : table.getTupleType(true).getFields() ) { final AlgDataTypeField targetField = logicalTargetRowType.getField( field.getName(), true, false ); switch ( strategies.get( field.getIndex() ) ) { case NOT_NULLABLE: diff --git a/plugins/sql-language/src/main/java/org/polypheny/db/sql/sql2alg/SqlToAlgConverter.java b/plugins/sql-language/src/main/java/org/polypheny/db/sql/sql2alg/SqlToAlgConverter.java index e325d74fb2..a64fb1f5a1 100644 --- a/plugins/sql-language/src/main/java/org/polypheny/db/sql/sql2alg/SqlToAlgConverter.java +++ b/plugins/sql-language/src/main/java/org/polypheny/db/sql/sql2alg/SqlToAlgConverter.java @@ -2895,7 +2895,7 @@ private RexNode castNullLiteralIfNeeded( RexNode node, AlgDataType type ) { */ protected void collectInsertTargets( SqlInsert call, final RexNode sourceRef, final List targetColumnNames, List columnExprs ) { final Entity targetTable = getTargetTable( call ); - final AlgDataType tableRowType = targetTable.getTupleType(); + final AlgDataType tableRowType = targetTable.getTupleType(true); SqlNodeList targetColumnList = call.getTargetColumnList(); if ( targetColumnList == null ) { if ( validator.getConformance().isInsertSubsetColumnsAllowed() ) {