diff --git a/go/libraries/doltcore/merge/mutable_secondary_index.go b/go/libraries/doltcore/merge/mutable_secondary_index.go index c8f02670b97..569f9de799c 100644 --- a/go/libraries/doltcore/merge/mutable_secondary_index.go +++ b/go/libraries/doltcore/merge/mutable_secondary_index.go @@ -85,7 +85,7 @@ func GetMutableSecondaryIdxsWithPending(ctx *sql.Context, ns tree.NodeStore, our idxKeyDesc = idxKeyDesc.PrefixDesc(idxKeyDesc.Count() - 1) } - if !idxKeyDesc.Equals(index.Schema().GetKeyDescriptorWithNoConversion(ns)) { + if !idxKeyDesc.Equals(index.Schema().GetKeyDescriptor(ns)) { continue } diff --git a/go/libraries/doltcore/merge/violations_fk.go b/go/libraries/doltcore/merge/violations_fk.go index d49dc3dcd56..0e9206fb703 100644 --- a/go/libraries/doltcore/merge/violations_fk.go +++ b/go/libraries/doltcore/merge/violations_fk.go @@ -67,9 +67,15 @@ type FKViolationReceiver interface { ProllyFKViolationFound(ctx context.Context, rowKey, rowValue val.Tuple) error } -// GetForeignKeyViolations returns the violations that have been created as a -// result of the diff between |baseRoot| and |newRoot|. It sends the violations to |receiver|. -func GetForeignKeyViolations(ctx *sql.Context, tableResolver doltdb.TableResolver, newRoot, baseRoot doltdb.RootValue, tables *doltdb.TableNameSet, receiver FKViolationReceiver) error { +// RegisterForeignKeyViolations emits constraint violations that have been created as a +// result of the diff between |baseRoot| and |newRoot|. It sends violations to |receiver|. +func RegisterForeignKeyViolations( + ctx *sql.Context, + tableResolver doltdb.TableResolver, + newRoot, baseRoot doltdb.RootValue, + tables *doltdb.TableNameSet, + receiver FKViolationReceiver, +) error { fkColl, err := newRoot.GetForeignKeyCollection(ctx) if err != nil { return err @@ -158,7 +164,7 @@ func GetForeignKeyViolations(ctx *sql.Context, tableResolver doltdb.TableResolve // todo(andy): pass doltdb.Rootish func AddForeignKeyViolations(ctx *sql.Context, tableResolver doltdb.TableResolver, newRoot, baseRoot doltdb.RootValue, tables *doltdb.TableNameSet, theirRootIsh hash.Hash) (doltdb.RootValue, *doltdb.TableNameSet, error) { violationWriter := &foreignKeyViolationWriter{tableResolver: tableResolver, rootValue: newRoot, theirRootIsh: theirRootIsh, violatedTables: doltdb.NewTableNameSet(nil)} - err := GetForeignKeyViolations(ctx, tableResolver, newRoot, baseRoot, tables, violationWriter) + err := RegisterForeignKeyViolations(ctx, tableResolver, newRoot, baseRoot, tables, violationWriter) if err != nil { return nil, nil, err } @@ -169,7 +175,7 @@ func AddForeignKeyViolations(ctx *sql.Context, tableResolver doltdb.TableResolve // violations based on the diff between |newRoot| and |baseRoot|. func GetForeignKeyViolatedTables(ctx *sql.Context, tableResolver doltdb.TableResolver, newRoot, baseRoot doltdb.RootValue, tables *doltdb.TableNameSet) (*doltdb.TableNameSet, error) { handler := &foreignKeyViolationTracker{tableSet: doltdb.NewTableNameSet(nil)} - err := GetForeignKeyViolations(ctx, tableResolver, newRoot, baseRoot, tables, handler) + err := RegisterForeignKeyViolations(ctx, tableResolver, newRoot, baseRoot, tables, handler) if err != nil { return nil, err } @@ -426,6 +432,7 @@ func childFkConstraintViolations( if err != nil { return err } + return prollyChildSecDiffFkConstraintViolations(ctx, foreignKey, postParent, postChild, m, receiver) } @@ -689,7 +696,13 @@ func childFkConstraintViolationsProcess( // newConstraintViolationsLoadedTable returns a *constraintViolationsLoadedTable. Returns false if the table was loaded // but the index could not be found. If the table could not be found, then an error is returned. -func newConstraintViolationsLoadedTable(ctx *sql.Context, tableResolver doltdb.TableResolver, tblName doltdb.TableName, idxName string, root doltdb.RootValue) (*constraintViolationsLoadedTable, bool, error) { +func newConstraintViolationsLoadedTable( + ctx *sql.Context, + tableResolver doltdb.TableResolver, + tblName doltdb.TableName, + idxName string, + root doltdb.RootValue, +) (*constraintViolationsLoadedTable, bool, error) { trueTblName, tbl, ok, err := tableResolver.ResolveTableInsensitive(ctx, root, tblName) if err != nil { return nil, false, err diff --git a/go/libraries/doltcore/merge/violations_fk_prolly.go b/go/libraries/doltcore/merge/violations_fk_prolly.go index f6a4bdc1201..df0b6440ab3 100644 --- a/go/libraries/doltcore/merge/violations_fk_prolly.go +++ b/go/libraries/doltcore/merge/violations_fk_prolly.go @@ -51,22 +51,39 @@ func prollyParentSecDiffFkConstraintViolations( return err } - parentSecKD, _ := postParentSecIdx.Descriptors() - parentPrefixKD := parentSecKD.PrefixDesc(len(foreignKey.TableColumns)) - partialKB := val.NewTupleBuilder(parentPrefixKD, postParentRowData.NodeStore()) + parentSecIdxDesc, _ := postParentSecIdx.Descriptors() + parentIdxPrefixDesc := parentSecIdxDesc.PrefixDesc(len(foreignKey.TableColumns)) + parentIdxKb := val.NewTupleBuilder(parentIdxPrefixDesc, postParentRowData.NodeStore()) childPriIdx, err := durable.ProllyMapFromIndex(postChild.RowData) if err != nil { return err } - childPriKD, _ := childPriIdx.Descriptors() + + childPriIdxDesc, _ := childPriIdx.Descriptors() + childSecIdxDesc, _ := childSecIdx.Descriptors() + + childPrimary := &indexAndKeyDescriptor{ + index: childPriIdx, + keyDesc: childPriIdxDesc, + schema: postChild.Schema, + } + childSecondary := &indexAndKeyDescriptor{ + index: childSecIdx, + keyDesc: childSecIdxDesc, + schema: postChild.IndexSchema, + } + + // We allow foreign keys between types that don't have the same serialization bytes for the same logical values + // in some contexts. If this lookup is one of those, we need to convert the child key to the parent key format. + compatibleTypes := foreignKeysAreCompatibleTypes(parentIdxPrefixDesc, childSecIdxDesc) // TODO: Determine whether we should surface every row as a diff when the map's value descriptor has changed. considerAllRowsModified := false err = prolly.DiffMaps(ctx, preParentSecIdx, postParentSecIdx, considerAllRowsModified, func(ctx context.Context, diff tree.Diff) error { switch diff.Type { case tree.RemovedDiff, tree.ModifiedDiff: - toSecKey, hadNulls, err := makePartialKey(partialKB, foreignKey.ReferencedTableColumns, postParent.Index, postParent.IndexSchema, val.Tuple(diff.Key), val.Tuple(diff.From), preParentSecIdx.Pool()) + k, hadNulls, err := makePartialKey(parentIdxKb, foreignKey.ReferencedTableColumns, postParent.Index, postParent.IndexSchema, val.Tuple(diff.Key), val.Tuple(diff.From), preParentSecIdx.Pool()) if err != nil { return err } @@ -75,7 +92,7 @@ func prollyParentSecDiffFkConstraintViolations( return nil } - ok, err := postParentSecIdx.HasPrefix(ctx, toSecKey, parentPrefixKD) + ok, err := postParentSecIdx.HasPrefix(ctx, k, parentIdxPrefixDesc) if err != nil { return err } @@ -85,7 +102,15 @@ func prollyParentSecDiffFkConstraintViolations( // All equivalent parents were deleted, let's check for dangling children. // We search for matching keys in the child's secondary index - err = createCVsForPartialKeyMatches(ctx, toSecKey, parentPrefixKD, childPriKD, childPriIdx, childSecIdx, postParentRowData.Pool(), receiver) + err = createCVsForDanglingChildRows( + ctx, + k, + parentIdxPrefixDesc, + childPrimary, + childSecondary, + receiver, + compatibleTypes, + ) if err != nil { return err } @@ -129,7 +154,24 @@ func prollyParentPriDiffFkConstraintViolations( if err != nil { return err } - primaryKD, _ := childPriIdx.Descriptors() + + childPriIdxDesc, _ := childPriIdx.Descriptors() + childSecIdxDesc, _ := childScndryIdx.Descriptors() + + childPrimary := &indexAndKeyDescriptor{ + index: childPriIdx, + keyDesc: childPriIdxDesc, + schema: postChild.Schema, + } + childSecondary := &indexAndKeyDescriptor{ + index: childScndryIdx, + keyDesc: childSecIdxDesc, + schema: postChild.IndexSchema, + } + + // We allow foreign keys between types that don't have the same serialization bytes for the same logical values + // in some contexts. If this lookup is one of those, we need to convert the child key to the parent key format. + compatibleTypes := foreignKeysAreCompatibleTypes(partialDesc, childSecIdxDesc) // TODO: Determine whether we should surface every row as a diff when the map's value descriptor has changed. considerAllRowsModified := false @@ -162,7 +204,15 @@ func prollyParentPriDiffFkConstraintViolations( // All equivalent parents were deleted, let's check for dangling children. // We search for matching keys in the child's secondary index - err = createCVsForPartialKeyMatches(ctx, partialKey, partialDesc, primaryKD, childPriIdx, childScndryIdx, childPriIdx.Pool(), receiver) + err = createCVsForDanglingChildRows( + ctx, + partialKey, + partialDesc, + childPrimary, + childSecondary, + receiver, + compatibleTypes, + ) if err != nil { return err } @@ -191,14 +241,19 @@ func prollyChildPriDiffFkConstraintViolations( if err != nil { return err } - parentScndryIdx, err := durable.ProllyMapFromIndex(postParent.IndexData) + parentSecondaryIdx, err := durable.ProllyMapFromIndex(postParent.IndexData) if err != nil { return err } - idxDesc, _ := parentScndryIdx.Descriptors() - partialDesc := idxDesc.PrefixDesc(len(foreignKey.TableColumns)) - partialKB := val.NewTupleBuilder(partialDesc, postChildRowData.NodeStore()) + childPriIdxDesc, _ := postChildRowData.Descriptors() + parentIdxDesc, _ := parentSecondaryIdx.Descriptors() + parentIdxPrefixDesc := parentIdxDesc.PrefixDesc(len(foreignKey.TableColumns)) + partialKB := val.NewTupleBuilder(parentIdxPrefixDesc, postChildRowData.NodeStore()) + + // We allow foreign keys between types that don't have the same serialization bytes for the same logical values + // in some contexts. If this lookup is one of those, we need to convert the child key to the parent key format. + compatibleTypes := foreignKeysAreCompatibleTypes(childPriIdxDesc, parentIdxPrefixDesc) // TODO: Determine whether we should surface every row as a diff when the map's value descriptor has changed. considerAllRowsModified := false @@ -206,7 +261,7 @@ func prollyChildPriDiffFkConstraintViolations( switch diff.Type { case tree.AddedDiff, tree.ModifiedDiff: k, v := val.Tuple(diff.Key), val.Tuple(diff.To) - partialKey, hasNulls, err := makePartialKey( + parentLookupKey, hasNulls, err := makePartialKey( partialKB, foreignKey.TableColumns, postChild.Index, @@ -221,7 +276,14 @@ func prollyChildPriDiffFkConstraintViolations( return nil } - err = createCVIfNoPartialKeyMatchesPri(ctx, k, v, partialKey, partialDesc, parentScndryIdx, receiver) + if !compatibleTypes { + parentLookupKey, err = convertKeyBetweenTypes(ctx, parentLookupKey, childPriIdxDesc, parentIdxPrefixDesc, parentSecondaryIdx.NodeStore(), parentSecondaryIdx.Pool()) + if err != nil { + return err + } + } + + err = createCVIfNoPartialKeyMatchesPri(ctx, k, v, parentLookupKey, parentIdxPrefixDesc, parentSecondaryIdx, receiver) if err != nil { return err } @@ -244,6 +306,7 @@ func prollyChildSecDiffFkConstraintViolations( postParent, postChild *constraintViolationsLoadedTable, preChildSecIdx prolly.Map, receiver FKViolationReceiver) error { + postChildRowData, err := durable.ProllyMapFromIndex(postChild.RowData) if err != nil { return err @@ -258,28 +321,42 @@ func prollyChildSecDiffFkConstraintViolations( } parentSecIdxDesc, _ := parentSecIdx.Descriptors() - prefixDesc := parentSecIdxDesc.PrefixDesc(len(foreignKey.TableColumns)) + parentIdxPrefixDesc := parentSecIdxDesc.PrefixDesc(len(foreignKey.TableColumns)) childPriKD, _ := postChildRowData.Descriptors() - childPriKB := val.NewTupleBuilder(childPriKD, preChildSecIdx.NodeStore()) + childIdxDesc, _ := postChildSecIdx.Descriptors() + childIdxPrefixDesc := childIdxDesc.PrefixDesc(len(foreignKey.TableColumns)) + + // We allow foreign keys between types that don't have the same serialization bytes for the same logical values + // in some contexts. If this lookup is one of those, we need to convert the child key to the parent key format. + compatibleTypes := foreignKeysAreCompatibleTypes(childIdxPrefixDesc, parentIdxPrefixDesc) // TODO: Determine whether we should surface every row as a diff when the map's value descriptor has changed. considerAllRowsModified := false err = prolly.DiffMaps(ctx, preChildSecIdx, postChildSecIdx, considerAllRowsModified, func(ctx context.Context, diff tree.Diff) error { switch diff.Type { case tree.AddedDiff, tree.ModifiedDiff: - k := val.Tuple(diff.Key) + key := val.Tuple(diff.Key) + parentLookupKey := key + // TODO: possible to skip this if there are not null constraints over entire index - for i := 0; i < k.Count(); i++ { - if k.FieldIsNull(i) { + for i := 0; i < parentLookupKey.Count(); i++ { + if parentLookupKey.FieldIsNull(i) { return nil } } - ok, err := parentSecIdx.HasPrefix(ctx, k, prefixDesc) + if !compatibleTypes { + parentLookupKey, err = convertKeyBetweenTypes(ctx, key, childIdxPrefixDesc, parentIdxPrefixDesc, postChildSecIdx.NodeStore(), postChildSecIdx.Pool()) + if err != nil { + return err + } + } + + ok, err := parentSecIdx.HasPrefix(ctx, parentLookupKey, parentIdxPrefixDesc) if err != nil { return err } else if !ok { - return createCVForSecIdx(ctx, k, childPriKD, childPriKB, postChildRowData, postChildRowData.Pool(), receiver) + return createCVForSecIdx(ctx, key, childPriKD, postChildRowData, postChild.Schema, postChild.IndexSchema, receiver) } return nil @@ -295,6 +372,69 @@ func prollyChildSecDiffFkConstraintViolations( return nil } +// foreignKeysAreCompatibleTypes returns whether the serializations for two tuple descriptors are binary compatible +func foreignKeysAreCompatibleTypes(keyDescA, keyDescB *val.TupleDesc) bool { + compatibleTypes := true + for i, handlerA := range keyDescA.Handlers { + handlerB := keyDescB.Handlers[i] + if handlerA != nil && handlerB != nil && !handlerA.SerializationCompatible(handlerB) { + compatibleTypes = false + break + } + } + return compatibleTypes +} + +// convertSerializedFkField converts a serialized foreign key value from one type handler to another. +func convertSerializedFkField( + ctx context.Context, + toHandler, fromHandler val.TupleTypeHandler, + field []byte, +) ([]byte, error) { + convertingHandler := toHandler + convertedHandler := fromHandler + + switch h := toHandler.(type) { + case val.AdaptiveEncodingTypeHandler: + convertingHandler = h.ChildHandler() + case val.AddressTypeHandler: + convertingHandler = h.ChildHandler() + } + + switch h := fromHandler.(type) { + case val.AdaptiveEncodingTypeHandler: + convertedHandler = h.ChildHandler() + adaptiveVal := val.AdaptiveValue(field) + if adaptiveVal.IsOutOfBand() { + var err error + field, err = h.ConvertToInline(ctx, adaptiveVal) + if err != nil { + return nil, err + } + } + field = field[1:] + case val.AddressTypeHandler: + convertedHandler = h.ChildHandler() + unhashed, err := h.DeserializeValue(ctx, field) + if err != nil { + return nil, err + } + + serialized, err := h.ChildHandler().SerializeValue(ctx, unhashed) + if err != nil { + return nil, err + } + + field = serialized + } + + serialized, err := convertingHandler.ConvertSerialized(ctx, convertedHandler, field) + if err != nil { + return nil, err + } + return serialized, nil +} + func createCVIfNoPartialKeyMatchesPri( ctx context.Context, k, v, partialKey val.Tuple, @@ -320,72 +460,116 @@ func createCVForSecIdx( ctx context.Context, k val.Tuple, primaryKD *val.TupleDesc, - primaryKb *val.TupleBuilder, pri prolly.Map, - pool pool.BuffPool, - receiver FKViolationReceiver) error { + tableSchema, indexSchema schema.Schema, + receiver FKViolationReceiver, +) error { // convert secondary idx entry to primary row key - // the pks of the table are the last keys of the index - o := k.Count() - primaryKD.Count() - for i := 0; i < primaryKD.Count(); i++ { - j := o + i - primaryKb.PutRaw(i, k.GetField(j)) - } - primaryIdxKey, err := primaryKb.Build(pool) + primaryKey, err := primaryKeyFromSecondaryIndexRow(k, primaryKD, pri, tableSchema, indexSchema) if err != nil { return err } var value val.Tuple - err = pri.Get(ctx, primaryIdxKey, func(k, v val.Tuple) error { + err = pri.Get(ctx, primaryKey, func(k, v val.Tuple) error { value = v return nil }) if err != nil { return err } + if value == nil { + return fmt.Errorf("unable to find row from secondary index in the primary index with key: %v", primaryKD.Format(ctx, primaryKey)) + } + + return receiver.ProllyFKViolationFound(ctx, primaryKey, value) +} + +func primaryKeyFromSecondaryIndexRow(secIndexRow val.Tuple, primaryKD *val.TupleDesc, pri prolly.Map, tableSchema schema.Schema, indexSchema schema.Schema) (val.Tuple, error) { + keyMap := makeOrdinalMappingForSchemas(indexSchema, tableSchema) + + kb := val.NewTupleBuilder(primaryKD, pri.NodeStore()) + for to := range keyMap { + from := keyMap.MapOrdinal(to) + kb.PutRaw(to, secIndexRow.GetField(from)) + } - return receiver.ProllyFKViolationFound(ctx, primaryIdxKey, value) + return kb.Build(pri.Pool()) } -func createCVsForPartialKeyMatches( +// makeOrdinalMappingForSchemas creates an ordinal mapping from one schema to another based on column names. +func makeOrdinalMappingForSchemas(fromSch, toSch schema.Schema) (m val.OrdinalMapping) { + from := fromSch.GetPKCols() + to := toSch.GetPKCols() + + // offset accounts for a keyless to schema, where the pseudo key column is the final one + offset := 0 + if from.Size() == 0 { + from = fromSch.GetNonPKCols() + } + if to.Size() == 0 { + to = toSch.GetNonPKCols() + offset = to.Size() + } + + m = make(val.OrdinalMapping, to.StoredSize()) + for i := range m { + col := to.GetByStoredIndex(i) + name := col.Name + colIdx := from.IndexOf(name) + m[i] = colIdx + offset + } + return +} + +type indexAndKeyDescriptor struct { + index prolly.Map + keyDesc *val.TupleDesc + schema schema.Schema +} + +// createCVsForDanglingChildRows finds all rows in the childIdx that match the given parent key and creates constraint +// violations for each of them using the provided receiver. +func createCVsForDanglingChildRows( ctx context.Context, partialKey val.Tuple, partialKeyDesc *val.TupleDesc, - primaryKD *val.TupleDesc, - primaryIdx prolly.Map, - secondaryIdx prolly.Map, - pool pool.BuffPool, + childPrimaryIdx *indexAndKeyDescriptor, + childSecIdx *indexAndKeyDescriptor, receiver FKViolationReceiver, + compatibleTypes bool, ) error { - itr, err := creation.NewPrefixItr(ctx, partialKey, partialKeyDesc, secondaryIdx) + // We allow foreign keys between types that don't have the same serialization bytes for the same logical values + // in some contexts. If this lookup is one of those, we need to convert the parent key to the child key format. + secondaryIndexKeyDesc := childSecIdx.keyDesc.PrefixDesc(partialKeyDesc.Count()) + if !compatibleTypes { + var err error + partialKey, err = convertKeyBetweenTypes(ctx, partialKey, partialKeyDesc, secondaryIndexKeyDesc, childSecIdx.index.NodeStore(), childPrimaryIdx.index.Pool()) + if err != nil { + return err + } + } + + itr, err := creation.NewPrefixItr(ctx, partialKey, secondaryIndexKeyDesc, childSecIdx.index) if err != nil { return err } - kb := val.NewTupleBuilder(primaryKD, primaryIdx.NodeStore()) - for k, _, err := itr.Next(ctx); err != io.EOF; k, _, err = itr.Next(ctx) { if err != nil { return err } // convert secondary idx entry to primary row key - // the pks of the table are the last keys of the index - o := k.Count() - primaryKD.Count() - for i := 0; i < primaryKD.Count(); i++ { - j := o + i - kb.PutRaw(i, k.GetField(j)) - } - primaryIdxKey, err := kb.Build(pool) + primaryIdxKey, err := primaryKeyFromSecondaryIndexRow(k, childPrimaryIdx.keyDesc, childPrimaryIdx.index, childPrimaryIdx.schema, childSecIdx.schema) if err != nil { return err } var value val.Tuple - err = primaryIdx.Get(ctx, primaryIdxKey, func(k, v val.Tuple) error { + err = childPrimaryIdx.index.Get(ctx, primaryIdxKey, func(k, v val.Tuple) error { value = v return nil }) @@ -397,7 +581,7 @@ func createCVsForPartialKeyMatches( // that can't be found in the primary index. This is never expected, so // we return an error. if value == nil { - return fmt.Errorf("unable to find row from secondary index in the primary index, with key: %v", primaryIdxKey) + return fmt.Errorf("unable to find row from secondary index in the primary index, with key: %v", childPrimaryIdx.keyDesc.Format(ctx, primaryIdxKey)) } // If a value was found, then there is a row in the child table that references the @@ -411,6 +595,53 @@ func createCVsForPartialKeyMatches( return nil } +// convertKeyBetweenTypes converts a partial key from one tuple descriptor's types to another. This is only necessary +// when the keys are of different types that are not serialization identical. +func convertKeyBetweenTypes( + ctx context.Context, + key val.Tuple, + fromKeyDesc *val.TupleDesc, + toKeyDesc *val.TupleDesc, + ns tree.NodeStore, + pool pool.BuffPool, +) (val.Tuple, error) { + tb := val.NewTupleBuilder(toKeyDesc, ns) + for i, fromHandler := range fromKeyDesc.Handlers { + toHandler := toKeyDesc.Handlers[i] + serialized, err := convertSerializedFkField(ctx, toHandler, fromHandler, key.GetField(i)) + if err != nil { + return nil, err + } + + switch toHandler.(type) { + case val.AdaptiveEncodingTypeHandler: + switch toKeyDesc.Types[i].Enc { + case val.ExtendedAdaptiveEnc: + err := tb.PutAdaptiveExtendedFromInline(ctx, i, serialized) + if err != nil { + return nil, err + } + case val.BytesAdaptiveEnc: + err := tb.PutAdaptiveExtendedFromInline(ctx, i, serialized) + if err != nil { + return nil, err + } + default: + panic(fmt.Sprintf("unexpected encoding for adaptive type: %d", fromKeyDesc.Types[i].Enc)) + } + default: + tb.PutRaw(i, serialized) + } + } + + var err error + key, err = tb.Build(pool) + if err != nil { + return nil, err + } + return key, nil +} + func makePartialKey(kb *val.TupleBuilder, tags []uint64, idxSch schema.Index, tblSch schema.Schema, k, v val.Tuple, pool pool.BuffPool) (val.Tuple, bool, error) { // Possible that the parent index (idxSch) is longer than the partial key (tags). if idxSch.Name() != "" && len(idxSch.IndexedColumnTags()) <= len(tags) { diff --git a/go/libraries/doltcore/schema/schema_impl.go b/go/libraries/doltcore/schema/schema_impl.go index c326a5a35fc..86862e0a436 100644 --- a/go/libraries/doltcore/schema/schema_impl.go +++ b/go/libraries/doltcore/schema/schema_impl.go @@ -476,9 +476,9 @@ func (si *schemaImpl) getKeyColumnsDescriptor(vs val.ValueStore, convertAddressC var handler val.TupleTypeHandler _, contentHashedField := contentHashedFields[tag] - extendedType, isExtendedType := sqlType.(sql.ExtendedType) + typeHandler, hasTypeHandler := sqlType.(val.TupleTypeHandler) - if isExtendedType { + if hasTypeHandler { encoding := EncodingFromSqlType(sqlType) t = val.Type{ Enc: val.Encoding(encoding), @@ -486,12 +486,12 @@ func (si *schemaImpl) getKeyColumnsDescriptor(vs val.ValueStore, convertAddressC } switch encoding { case serial.EncodingExtendedAddr: - handler = val.NewExtendedAddressTypeHandler(vs, extendedType) + handler = val.NewExtendedAddressTypeHandler(vs, typeHandler) case serial.EncodingExtendedAdaptive: - handler = val.NewAdaptiveTypeHandler(vs, extendedType) + handler = val.NewAdaptiveTypeHandler(vs, typeHandler) default: // encoding == serial.EncodingExtended - handler = extendedType + handler = typeHandler } } else { if convertAddressColumns && !contentHashedField && queryType == query.Type_BLOB { @@ -573,15 +573,15 @@ func (si *schemaImpl) GetValueDescriptor(vs val.ValueStore) *val.TupleDesc { collations = append(collations, sql.Collation_Unspecified) } - if extendedType, ok := sqlType.(sql.ExtendedType); ok { + if typeHandler, ok := sqlType.(val.TupleTypeHandler); ok { switch encoding { case serial.EncodingExtendedAddr: - handlers = append(handlers, val.NewExtendedAddressTypeHandler(vs, extendedType)) + handlers = append(handlers, val.NewExtendedAddressTypeHandler(vs, typeHandler)) case serial.EncodingExtendedAdaptive: - handlers = append(handlers, val.NewAdaptiveTypeHandler(vs, extendedType)) + handlers = append(handlers, val.NewAdaptiveTypeHandler(vs, typeHandler)) default: // encoding == serial.EncodingExtended - handlers = append(handlers, extendedType) + handlers = append(handlers, typeHandler) } } else { handlers = append(handlers, nil) diff --git a/go/libraries/doltcore/sqle/enginetest/dolt_engine_test.go b/go/libraries/doltcore/sqle/enginetest/dolt_engine_test.go index ccacb340aed..0e0dffc0049 100644 --- a/go/libraries/doltcore/sqle/enginetest/dolt_engine_test.go +++ b/go/libraries/doltcore/sqle/enginetest/dolt_engine_test.go @@ -116,50 +116,33 @@ func TestSingleScript(t *testing.T) { t.Skip() var scripts = []queries.ScriptTest{ { - Name: "Database syntax properly handles inter-CALL communication", + Name: "foreign key violation, parent row deleted, child keyless", SetUpScript: []string{ - `CREATE PROCEDURE p1() -BEGIN - DECLARE str VARCHAR(20); - CALL p2(str); - SET str = CONCAT('a', str); - SELECT str; -END`, - `CREATE PROCEDURE p2(OUT param VARCHAR(20)) -BEGIN - SET param = 'b'; -END`, - "CALL DOLT_ADD('-A');", - "CALL DOLT_COMMIT('-m', 'First procedures');", - "CALL DOLT_BRANCH('p12');", - "DROP PROCEDURE p1;", - "DROP PROCEDURE p2;", - `CREATE PROCEDURE p1() -BEGIN - DECLARE str VARCHAR(20); - CALL p2(str); - SET str = CONCAT('c', str); - SELECT str; -END`, - `CREATE PROCEDURE p2(OUT param VARCHAR(20)) -BEGIN - SET param = 'd'; -END`, - "CALL DOLT_ADD('-A');", - "CALL DOLT_COMMIT('-m', 'Second procedures');", + "create table parent (pk int primary key, a int);", + "create index idx_a on parent(pk, a);", + "create table child (fk1 int, fk2 int, foreign key (fk1, fk2) references parent(pk, a));", + "insert into parent values (10, 1), (20, 2);", + "insert into child values (10, 1), (20, 2);", + "call dolt_commit('-Am', 'setup');", + "call dolt_branch('other');", + "delete from child where fk2 = 1;", + "delete from parent where pk = 10;", + "call dolt_commit('-am', 'delete parent and child rows');", + "call dolt_checkout('other');", + "insert into child values (10, 1);", + "call dolt_commit('-am', 'insert child row');", + "set dolt_force_transaction_commit = on;", }, Assertions: []queries.ScriptTestAssertion{ { - Query: "CALL p1();", - Expected: []sql.Row{{"cd"}}, - }, - { - Query: "CALL `mydb/main`.p1();", - Expected: []sql.Row{{"cd"}}, + Query: "call dolt_merge('main')", + SkipResultsCheck: true, }, { - Query: "CALL `mydb/p12`.p1();", - Expected: []sql.Row{{"ab"}}, + Query: "select violation_type, fk2 from dolt_constraint_violations_child", + Expected: []sql.Row{ + {"foreign key", 1}, + }, }, }, }, diff --git a/go/libraries/doltcore/sqle/enginetest/dolt_queries_merge.go b/go/libraries/doltcore/sqle/enginetest/dolt_queries_merge.go index c18b00a5968..e20ab78e31d 100644 --- a/go/libraries/doltcore/sqle/enginetest/dolt_queries_merge.go +++ b/go/libraries/doltcore/sqle/enginetest/dolt_queries_merge.go @@ -3075,6 +3075,140 @@ var MergeScripts = []queries.ScriptTest{ }, }, }, + { + Name: "foreign key violation, parent row deleted", + SetUpScript: []string{ + "create table parent (pk int primary key);", + "create table child (fk int, foreign key (fk) references parent(pk));", + "insert into parent values (1), (2);", + "insert into child values (1), (2);", + "call dolt_commit('-Am', 'setup');", + "call dolt_branch('other');", + "delete from child where fk = 1;", + "delete from parent where pk = 1;", + "call dolt_commit('-am', 'delete parent and child rows');", + "call dolt_checkout('other');", + "insert into child values (1);", + "call dolt_commit('-am', 'insert child row');", + "set dolt_force_transaction_commit = on;", + }, + Assertions: []queries.ScriptTestAssertion{ + { + Query: "call dolt_merge('main')", + SkipResultsCheck: true, + }, + { + Query: "select violation_type, fk from dolt_constraint_violations_child", + Expected: []sql.Row{ + {"foreign key", 1}, + }, + }, + }, + }, + { + Name: "foreign key violation, parent row deleted, child keyless", + SetUpScript: []string{ + "create table parent (pk int primary key, a int);", + "create index idx_a on parent(pk, a);", + "create table child (fk1 int, fk2 int, foreign key (fk1, fk2) references parent(pk, a));", + "insert into parent values (10, 1), (20, 2);", + "insert into child values (10, 1), (20, 2);", + "call dolt_commit('-Am', 'setup');", + "call dolt_branch('other');", + "delete from child where fk2 = 1;", + "delete from parent where pk = 10;", + "call dolt_commit('-am', 'delete parent and child rows');", + "call dolt_checkout('other');", + "insert into child values (10, 1);", + "call dolt_commit('-am', 'insert child row');", + "set dolt_force_transaction_commit = on;", + }, + Assertions: []queries.ScriptTestAssertion{ + { + Query: "call dolt_merge('main')", + SkipResultsCheck: true, + }, + { + Query: "select violation_type, fk2 from dolt_constraint_violations_child", + Expected: []sql.Row{ + {"foreign key", 1}, + }, + }, + }, + }, + { + Name: "foreign key violation, parent row deleted, secondary index", + SetUpScript: []string{ + "create table parent (pk int primary key, v1 int);", + "create index idx_v1 on parent(v1);", + "create table child (fk int, foreign key (fk) references parent(v1));", + "insert into parent values (1, 1), (2, 2);", + "insert into child values (1), (2);", + "call dolt_commit('-Am', 'setup');", + "call dolt_branch('other');", + "delete from child where fk = 1;", + "delete from parent where pk = 1;", + "call dolt_commit('-am', 'delete parent and child rows');", + "call dolt_checkout('other');", + "insert into child values (1);", + "call dolt_commit('-am', 'insert child row');", + "set dolt_force_transaction_commit = on;", + }, + Assertions: []queries.ScriptTestAssertion{ + { + Query: "call dolt_merge('main')", + SkipResultsCheck: true, + }, + { + Query: "select violation_type, fk from dolt_constraint_violations_child", + Expected: []sql.Row{ + {"foreign key", 1}, + }, + }, + }, + }, + { + Name: "Merge foreign keys, 2 column key, dangling child violation", + SetUpScript: []string{ + `CREATE TABLE parent ( + a VARCHAR(256), + b varchar(256), + c VARCHAR(256), + PRIMARY KEY (b, a) + );`, + `CREATE INDEX idx_parent_on_a_b ON parent (a, b);`, + `CREATE TABLE child ( + d VARCHAR(256), + e VARCHAR(256), + f varchar(256), + PRIMARY KEY (e, d), + CONSTRAINT child_fk FOREIGN KEY (d, f) REFERENCES parent (a, b) + );`, + `INSERT INTO parent VALUES ('abc','def', 'xyz');`, + `INSERT INTO child VALUES ('abc','123','def');`, + `CALL DOLT_COMMIT('-Am', '1');`, + `CALL DOLT_BRANCH('other_branch');`, + `INSERT INTO child VALUES ('abc','www','def');`, + `CALL DOLT_COMMIT('-Am', '2');`, + `CREATE TABLE table3 (table3_col1 VARCHAR(256));`, + `CALL DOLT_COMMIT('-Am', '3');`, + `CALL DOLT_CHECKOUT('other_branch');`, + `delete from child where e='123';`, + `delete from parent where a='abc';`, + `CALL DOLT_COMMIT('-Am', '4');`, + `set dolt_force_transaction_commit=1;`, + }, + Assertions: []queries.ScriptTestAssertion{ + { + Query: "CALL dolt_merge('main')", + Expected: []sql.Row{{"", 0, 1, "conflicts found"}}, + }, + { + Query: "select violation_type, d, e, f from dolt_constraint_violations_child;", + Expected: []sql.Row{{"foreign key", "abc", "www", "def"}}, + }, + }, + }, } var KeylessMergeCVsAndConflictsScripts = []queries.ScriptTest{ diff --git a/go/store/val/adaptive_value.go b/go/store/val/adaptive_value.go index 636a34948c3..1167da892e1 100644 --- a/go/store/val/adaptive_value.go +++ b/go/store/val/adaptive_value.go @@ -206,6 +206,28 @@ type AdaptiveEncodingTypeHandler struct { childHandler TupleTypeHandler } +func (handler AdaptiveEncodingTypeHandler) ChildHandler() TupleTypeHandler { + return handler.childHandler +} + +func (handler AdaptiveEncodingTypeHandler) SerializationCompatible(other TupleTypeHandler) bool { + _, ok := other.(AdaptiveEncodingTypeHandler) + return ok +} + +func (handler AdaptiveEncodingTypeHandler) ConvertSerialized(ctx context.Context, other TupleTypeHandler, val []byte) ([]byte, error) { + otherAdaptiveVal := AdaptiveValue(val) + // TODO: we don't know if this value is inlined or out-of-band. We need to check both. + outOfBand, err := otherAdaptiveVal.convertToOutOfBand(ctx, handler.vs, nil) + if err != nil { + return nil, err + } + + return handler.childHandler.ConvertSerialized(ctx, other, outOfBand[:]) +} + +var _ TupleTypeHandler = AdaptiveEncodingTypeHandler{} + func NewAdaptiveTypeHandler(vs ValueStore, childHandler TupleTypeHandler) AdaptiveEncodingTypeHandler { return AdaptiveEncodingTypeHandler{ vs: vs, @@ -213,6 +235,10 @@ func NewAdaptiveTypeHandler(vs ValueStore, childHandler TupleTypeHandler) Adapti } } +func (handler AdaptiveEncodingTypeHandler) ConvertToInline(ctx context.Context, val AdaptiveValue) ([]byte, error) { + return val.convertToInline(ctx, handler.vs, nil) +} + func (handler AdaptiveEncodingTypeHandler) SerializedCompare(ctx context.Context, v1 []byte, v2 []byte) (int, error) { // order NULLs first if v1 == nil || v2 == nil { @@ -317,7 +343,7 @@ func (e ExtendedValueWrapper) MaxByteLength() int64 { } func (e ExtendedValueWrapper) Compare(ctx context.Context, other interface{}) (cmp int, comparable bool, err error) { - //TODO implement me + // TODO implement me panic("implement me") } diff --git a/go/store/val/tuple_descriptor.go b/go/store/val/tuple_descriptor.go index 9fa3c425bc8..f37a393fcab 100644 --- a/go/store/val/tuple_descriptor.go +++ b/go/store/val/tuple_descriptor.go @@ -62,6 +62,10 @@ type TupleTypeHandler interface { DeserializeValue(ctx context.Context, val []byte) (any, error) // FormatValue returns a string version of the value. Primarily intended for display. FormatValue(val any) (string, error) + // SerializationCompatible returns true if this TupleTypeHandler is binary compatible with the other TupleTypeHandler. + SerializationCompatible(other TupleTypeHandler) bool + // ConvertSerialized converts |val| from the type handled by |other| to the type handled by this TupleTypeHandler. + ConvertSerialized(ctx context.Context, other TupleTypeHandler, val []byte) ([]byte, error) } // TupleDescriptorArgs are a set of optional arguments for TupleDesc creation. @@ -755,6 +759,19 @@ type AddressTypeHandler struct { childHandler TupleTypeHandler } +func (handler AddressTypeHandler) SerializationCompatible(other TupleTypeHandler) bool { + _, ok := other.(AddressTypeHandler) + return ok +} + +func (handler AddressTypeHandler) ConvertSerialized(ctx context.Context, other TupleTypeHandler, val []byte) ([]byte, error) { + panic("unexpected call to ConvertSerialized on AddressTypeHandler; this should never be necessary because" + + " AddressTypeHandler should only be used in contexts where the child handler's serialized form is never " + + "directly compared or converted") +} + +var _ TupleTypeHandler = AddressTypeHandler{} + func NewExtendedAddressTypeHandler(vs ValueStore, childHandler TupleTypeHandler) AddressTypeHandler { return AddressTypeHandler{ vs: vs, @@ -815,3 +832,7 @@ func (handler AddressTypeHandler) DeserializeValue(ctx context.Context, val []by func (handler AddressTypeHandler) FormatValue(val any) (string, error) { return handler.childHandler.FormatValue(val) } + +func (handler AddressTypeHandler) ChildHandler() TupleTypeHandler { + return handler.childHandler +}