Skip to content
Permalink
Browse files
IGNITE-16474 Forbid nullable key columns (#676)
  • Loading branch information
Taras Ledkov committed Feb 21, 2022
1 parent 3eb87bf commit 0a7aa64655ec1a77e1cce809c1354bda22a6479d
Show file tree
Hide file tree
Showing 26 changed files with 467 additions and 646 deletions.
@@ -65,6 +65,11 @@ public static void setLastVer(int lastVer) {
@Override
@NotNull
public SchemaDescriptor schema(int ver) {
if (ver == 0) {
// Use last version (any version may be used) for 0 version, that mean row doens't contain value.
ver = lastVer;
}

SchemaDescriptor desc = schemaCache.get(ver);

if (desc != null) {
@@ -0,0 +1,57 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one or more
* contributor license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright ownership.
* The ASF licenses this file to You under the Apache License, Version 2.0
* (the "License"); you may not use this file except in compliance with
* the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package org.apache.ignite.internal.sql.engine;

import static org.junit.jupiter.api.Assertions.assertThrows;

import org.apache.ignite.lang.IgniteException;
import org.junit.jupiter.api.Test;

/**
* Integration test for set op (EXCEPT, INTERSECT).
*/
public class ItCreateTableDdlTest extends AbstractBasicIntegrationTest {
@Test
public void pkWithNullableColumns() {
assertThrows(
IgniteException.class,
() -> sql("CREATE TABLE T0(ID0 INT NULL, ID1 INT NOT NULL, VAL INT, PRIMARY KEY (ID1, ID0))"),
"Primary key cannot contain nullable column [col=ID0]"
);
assertThrows(
IgniteException.class,
() -> sql("CREATE TABLE T0(ID INT NULL PRIMARY KEY, VAL INT)"),
"Primary key cannot contain nullable column [col=ID]"
);
}

@Test
public void undefinedColumnsInPrimaryKey() {
assertThrows(
IgniteException.class,
() -> sql("CREATE TABLE T0(ID INT, VAL INT, PRIMARY KEY (ID1, ID0, ID2))"),
" Primary key constrain contains undefined columns: [cols=[ID0, ID2, ID1]]"
);
}

@Test
public void dbg() {
sql("CREATE TABLE T0(ID INT, VAL INT, PRIMARY KEY (ID))");
}

}
@@ -25,20 +25,32 @@
* Binary row interface. The class contains low-level methods to read row data.
*/
public interface BinaryRow {
/** Size of chunk length field. */
int CHUNK_LEN_FLD_SIZE = Integer.BYTES;

/** Size of flags field. */
int FLAGS_FLD_SIZE = Byte.BYTES;

/** Size of schema version length field. */
int SCHEMA_VERSION_FLD_LEN = Short.BYTES;

/** Size of key hash field. */
int HASH_FLD_SIZE = Integer.BYTES;

/** Row schema version field offset. */
int SCHEMA_VERSION_OFFSET = 0;

/** Row flags field offset. */
int FLAGS_FIELD_OFFSET = SCHEMA_VERSION_OFFSET + 2 /* version length */;
/** Row flags field offset from the chunk start. */
int FLAGS_FIELD_OFFSET = CHUNK_LEN_FLD_SIZE;

/** Key hash field offset. */
int KEY_HASH_FIELD_OFFSET = FLAGS_FIELD_OFFSET + 2 /* flags length */;
int KEY_HASH_FIELD_OFFSET = SCHEMA_VERSION_FLD_LEN;

/** Key chunk field offset. */
int KEY_CHUNK_OFFSET = KEY_HASH_FIELD_OFFSET + 4 /* hash length */;
int KEY_CHUNK_OFFSET = KEY_HASH_FIELD_OFFSET + HASH_FLD_SIZE;

/** Size of chunk length field. */
int CHUNK_LEN_FLD_SIZE = Integer.BYTES;
/** Size of chunk header (offset for nullmap or vartable or data). */
int CHUNK_HEADER_SIZE = CHUNK_LEN_FLD_SIZE + FLAGS_FLD_SIZE;

/** Row header size. */
int HEADER_SIZE = KEY_CHUNK_OFFSET;
@@ -153,17 +165,8 @@ public interface BinaryRow {
* Row flags.
*/
final class RowFlags {
/** Flag indicates row has no value chunk. */
public static final int NO_VALUE_FLAG = 1;

/** Chunk flags mask. */
public static final int CHUNK_FLAGS_MASK = 0x0F;

/** Key specific flags. */
public static final int KEY_FLAGS_OFFSET = 8;

/** Value specific flags. */
public static final int VAL_FLAGS_OFFSET = 12;
/** First two flag bits reserved for format code. */
public static final int VARTABLE_FORMAT_MASK = 0x03;

/** Stub. */
private RowFlags() {
@@ -61,9 +61,9 @@ public int schemaVersion() {
/** {@inheritDoc} */
@Override
public boolean hasValue() {
short flags = readShort(FLAGS_FIELD_OFFSET);
short schemaVer = readShort(SCHEMA_VERSION_OFFSET);

return (flags & RowFlags.NO_VALUE_FLAG) == 0;
return schemaVer > 0;
}

/** {@inheritDoc} */
@@ -79,6 +79,8 @@ public SchemaDescriptor(int ver, Column[] keyCols, @Nullable String[] affCols, C
this.keyCols = new Columns(0, keyCols);
this.valCols = new Columns(keyCols.length, valCols);

assert this.keyCols.nullMapSize() == 0 : "Primary key cannot contain nullable column [cols=" + this.keyCols + ']';

colMap = new LinkedHashMap<>(keyCols.length + valCols.length);

Stream.concat(Arrays.stream(this.keyCols.columns()), Arrays.stream(this.valCols.columns()))
@@ -126,6 +126,7 @@ public TableDefinition build() {
assert primaryKeyDefinition != null : "Primary key index must be configured.";
assert columns.size() > primaryKeyDefinition.columns().size() : "Key or/and value columns must be defined.";

validatePrimaryKey(primaryKeyDefinition.columns(), columns);
validateIndices(indices.values(), columns.values(), primaryKeyDefinition.affinityColumns());

return new TableDefinitionImpl(
@@ -137,6 +138,21 @@ public TableDefinition build() {
);
}

/**
* Validate primary key.
*
* @param pkColNames Primary key columns.
* @param cols Table columns.
*/
private static void validatePrimaryKey(Set<String> pkColNames, final Map<String, ColumnDefinition> cols) {
pkColNames.stream()
.filter(pkCol -> cols.get(pkCol).nullable())
.findAny()
.ifPresent((pkCol) -> {
throw new IllegalStateException("Primary key cannot contain nullable column [col=" + pkCol + "].");
});
}

/**
* Validate indices.
*
@@ -82,6 +82,11 @@ public SchemaRegistryImpl(int initialVer, Function<Integer, SchemaDescriptor> hi
/** {@inheritDoc} */
@Override
public SchemaDescriptor schema(int ver) {
if (ver == 0) {
// Use last version (any version may be used) for 0 version, that mean row doens't contain value.
ver = lastVer;
}

SchemaDescriptor desc = schemaCache.get(ver);

if (desc != null) {
@@ -539,7 +539,8 @@ protected long findColumn(int colIdx, NativeTypeSpec type) throws InvalidTypeExc
chunkBaseOff = KEY_CHUNK_OFFSET;

flags = keyFlags();
} else { // Adjust the column index according to the number of key columns.
} else {
// Adjust the column index according to the number of key columns.
if (!hasValue()) {
throw new IllegalStateException("Row has no value.");
}
@@ -557,19 +558,17 @@ protected long findColumn(int colIdx, NativeTypeSpec type) throws InvalidTypeExc
throw new InvalidTypeException("Invalid column type requested [requested=" + type + ", column=" + cols.column(colIdx) + ']');
}

int nullMapLen = (flags & VarTableFormat.OMIT_NULL_MAP_FLAG) == 0 ? cols.nullMapSize() : 0;
int nullMapLen = cols.nullMapSize();

VarTableFormat format = (flags & VarTableFormat.OMIT_VARTBL_FLAG) == 0 ? VarTableFormat.fromFlags(flags) : null;
VarTableFormat format = VarTableFormat.fromFlags(flags);

if (nullMapLen > 0 && isNull(chunkBaseOff, colIdx)) {
return -1;
}

int dataOffset = varTableOffset(chunkBaseOff, nullMapLen);

if (format != null) {
dataOffset += format.vartableLength(format.readVartableSize(row, dataOffset));
}
dataOffset += format.vartableLength(format.readVartableSize(row, dataOffset));

return type.fixedLength()
? fixedSizeColumnOffset(chunkBaseOff, dataOffset, cols, colIdx, nullMapLen > 0) :
@@ -677,7 +676,7 @@ long varlenColumnOffsetAndLength(
int off = cols.numberOfFixsizeColumns() == 0 ? dataOff
: fixedSizeColumnOffset(baseOff, dataOff, cols, cols.numberOfFixsizeColumns(), nullMapLen > 0);

long len = format != null
long len = format != VarTableFormat.SKIPPED
?
// Length is either diff between current offset and next varlen offset or end-of-chunk.
dataOff + format.readVarlenOffset(row, varTableOffset(baseOff, nullMapLen), 0) - off :
@@ -724,7 +723,7 @@ private int chunkLength(int baseOff) {
* @return Vartable offset.
*/
private int varTableOffset(int baseOff, int nullMapLen) {
return baseOff + BinaryRow.CHUNK_LEN_FLD_SIZE + nullMapLen;
return baseOff + BinaryRow.CHUNK_HEADER_SIZE + nullMapLen;
}

/**
@@ -738,7 +737,7 @@ protected boolean isNull(int baseOff, int idx) {
int nullByte = idx >> 3; // Equivalent expression for: idx / 8
int posInByte = idx & 7; // Equivalent expression for: idx % 8

int map = row.readByte(baseOff + BinaryRow.CHUNK_LEN_FLD_SIZE + nullByte) & 0xFF;
int map = row.readByte(baseOff + BinaryRow.CHUNK_HEADER_SIZE + nullByte) & 0xFF;

return (map & (1 << posInByte)) != 0;
}
@@ -749,10 +748,8 @@ protected boolean isNull(int baseOff, int idx) {
*
* @return Key flags.
*/
private int keyFlags() {
short flags = readShort(FLAGS_FIELD_OFFSET);

return (flags >> RowFlags.KEY_FLAGS_OFFSET) & RowFlags.CHUNK_FLAGS_MASK;
private byte keyFlags() {
return readByte(KEY_CHUNK_OFFSET + FLAGS_FIELD_OFFSET);
}

/**
@@ -761,10 +758,10 @@ private int keyFlags() {
*
* @return Value flags.
*/
private int valueFlags() {
short flags = readShort(FLAGS_FIELD_OFFSET);
private byte valueFlags() {
int keyLen = readInteger(KEY_CHUNK_OFFSET);

return (flags >> RowFlags.VAL_FLAGS_OFFSET) & RowFlags.CHUNK_FLAGS_MASK;
return readByte(KEY_CHUNK_OFFSET + keyLen + FLAGS_FIELD_OFFSET);
}

/**
@@ -775,7 +772,7 @@ private int valueFlags() {
* @return Null-map offset.
*/
int nullMapOffset(int baseOff) {
return baseOff + BinaryRow.CHUNK_LEN_FLD_SIZE;
return baseOff + BinaryRow.CHUNK_LEN_FLD_SIZE + BinaryRow.FLAGS_FLD_SIZE;
}

/**
@@ -18,8 +18,6 @@
package org.apache.ignite.internal.schema.row;

import static org.apache.ignite.internal.schema.BinaryRow.KEY_CHUNK_OFFSET;
import static org.apache.ignite.internal.schema.BinaryRow.RowFlags.KEY_FLAGS_OFFSET;
import static org.apache.ignite.internal.schema.BinaryRow.RowFlags.VAL_FLAGS_OFFSET;

import java.math.BigDecimal;
import java.math.BigInteger;
@@ -35,7 +33,6 @@
import java.util.UUID;
import org.apache.ignite.internal.schema.AssemblyException;
import org.apache.ignite.internal.schema.BinaryRow;
import org.apache.ignite.internal.schema.BinaryRow.RowFlags;
import org.apache.ignite.internal.schema.BitmaskNativeType;
import org.apache.ignite.internal.schema.ByteBufferRow;
import org.apache.ignite.internal.schema.Column;
@@ -98,7 +95,7 @@ public class RowAssembler {
private int dataOff;

/** Flags. */
private short flags;
private byte flags;

/** Charset encoder for strings. Initialized lazily. */
private CharsetEncoder strEncoder;
@@ -734,16 +731,15 @@ private void flush() {
throw new AssemblyException("Key column missed: colIdx=" + curCol);
} else {
if (curCol == 0) {
flags &= ~(RowFlags.CHUNK_FLAGS_MASK << VAL_FLAGS_OFFSET);
flags |= RowFlags.NO_VALUE_FLAG;
// Row has no value
buf.putShort(BinaryRow.SCHEMA_VERSION_OFFSET, (short) 0);
} else if (schema.valueColumns().length() != curCol) {
throw new AssemblyException("Value column missed: colIdx=" + curCol);
}
}

int hash = HashUtils.hash32(buf.unwrap().array(), KEY_CHUNK_OFFSET, keyChunkLength, 0);

buf.putShort(BinaryRow.FLAGS_FIELD_OFFSET, flags);
buf.putInt(BinaryRow.KEY_HASH_FIELD_OFFSET, hash);
}

@@ -870,13 +866,14 @@ private void finishChunk() {

curOff -= format.compactVarTable(buf, varTblOff, curVartblEntry - 1);

flags |= format.formatId() << (isKeyChunk() ? KEY_FLAGS_OFFSET : VAL_FLAGS_OFFSET);
flags |= format.formatId();
}

// Write sizes.
final int chunkLen = curOff - baseOff;

buf.putInt(baseOff, chunkLen);
buf.put(baseOff + BinaryRow.FLAGS_FIELD_OFFSET, flags);

if (schema.keyColumns() == curCols) {
keyChunkLength = chunkLen;
@@ -910,23 +907,12 @@ private void switchToValueChunk(int baseOff) {
private void initChunk(int baseOff, int nullMapLen, int vartblLen) {
this.baseOff = baseOff;

nullMapOff = baseOff + BinaryRow.CHUNK_LEN_FLD_SIZE;
nullMapOff = baseOff + BinaryRow.CHUNK_LEN_FLD_SIZE + BinaryRow.FLAGS_FLD_SIZE;
varTblOff = nullMapOff + nullMapLen;
dataOff = varTblOff + vartblLen;
curOff = dataOff;
curVartblEntry = 0;

int flags = 0;

if (nullMapLen == 0) {
flags |= VarTableFormat.OMIT_NULL_MAP_FLAG;
}

if (vartblLen == 0) {
flags |= VarTableFormat.OMIT_VARTBL_FLAG;
}

this.flags |= flags << (isKeyChunk() ? KEY_FLAGS_OFFSET : VAL_FLAGS_OFFSET);
flags = 0;
}

/**

0 comments on commit 0a7aa64

Please sign in to comment.