Merge upstream/main into codex/issue-23143-copy-clone-auto-increment-reconcile

Author: VioletQwQ-0

cmd/mo-object-tool/ckp/checkpoint_test.go

@@ -189,6 +189,37 @@ func TestRestoreCreateTableDDLUsesExternalCreateSQL(t *testing.T) {
 	assert.Equal(t, "CREATE EXTERNAL TABLE `ckp_external`.`ext_csv_local` (\n  `id` INT\n) INFILE {'filepath'='/tmp/ext.csv', 'compression'='none', 'format'='csv'} FIELDS TERMINATED BY ',' ENCLOSED BY '\"' LINES TERMINATED BY '\\\\n' IGNORE 1 LINES", ddl)
 }
 
+func TestRestoreCreateTableDDLAcceptsUnicodeColumnComment(t *testing.T) {
+	table := checkpointtool.TableCatalogEntry{
+		TableID:      272864,
+		DatabaseName: "big_data_test",
+		TableName:    "ca_comprehensive_dataset",
+	}
+	data := &checkpointtool.TableDumpData{
+		TableID: table.TableID,
+		Schema: &checkpointtool.TableSchema{
+			TableName:    table.TableName,
+			DatabaseName: table.DatabaseName,
+			Columns: []checkpointtool.TableColumn{
+				{Name: "md5_id", SQLType: "VARCHAR(255)", NotNull: true},
+				{Name: "question_vector", SQLType: "VECF64(1024)", Comment: "摘要的向量集"},
+			},
+			PrimaryKey: []string{"md5_id"},
+		},
+	}
+
+	ddl, err := restoreCreateTableDDL(
+		context.Background(),
+		&checkpointtool.CheckpointReader{},
+		table,
+		data,
+		types.BuildTS(10, 0),
+	)
+	require.NoError(t, err)
+	require.Contains(t, ddl, "`question_vector` VECF64(1024)")
+	require.Contains(t, ddl, "COMMENT '摘要的向量集'")
+}
+
 func TestRestoreCreateTableDDLExternalRequiresCreateSQL(t *testing.T) {
 	table := checkpointtool.TableCatalogEntry{
 		DatabaseName: "ckp_external",

cmd/mo-tool/main.go

@@ -16,6 +16,7 @@ package main
 
 import (
 	"os"
+	"strings"
 
 	dashboard "github.com/matrixorigin/matrixone/cmd/mo-dashboard"
 	debug "github.com/matrixorigin/matrixone/cmd/mo-debug"
@@ -38,7 +39,46 @@ func main() {
 	rootCmd.AddCommand(object.PrepareCommand())
 	rootCmd.AddCommand(ckp.PrepareCommand())
 
+	rootCmd.SetArgs(normalizeLegacyRemoteS3Args(os.Args[1:]))
 	if err := rootCmd.Execute(); err != nil {
 		os.Exit(1)
 	}
 }
+
+// normalizeLegacyRemoteS3Args keeps ckp invocations from before --remote-s3
+// was introduced working without changing the local S3FS selector semantics of
+// a bare --s3 flag.
+func normalizeLegacyRemoteS3Args(args []string) []string {
+	normalized := append([]string(nil), args...)
+	if len(normalized) == 0 || normalized[0] != "ckp" {
+		return normalized
+	}
+
+	for i := 1; i < len(normalized); i++ {
+		switch {
+		case normalized[i] == "--s3" &&
+			i+1 < len(normalized) &&
+			looksLikeRemoteS3Arguments(normalized[i+1]):
+			normalized[i] = "--remote-s3"
+			i++
+		case strings.HasPrefix(normalized[i], "--s3=") &&
+			looksLikeRemoteS3Arguments(strings.TrimPrefix(normalized[i], "--s3=")):
+			normalized[i] = "--remote-s3=" + strings.TrimPrefix(normalized[i], "--s3=")
+		}
+	}
+
+	return normalized
+}
+
+func looksLikeRemoteS3Arguments(value string) bool {
+	for _, option := range strings.Split(value, ",") {
+		key, optionValue, ok := strings.Cut(strings.TrimSpace(option), "=")
+		if !ok || optionValue == "" {
+			continue
+		}
+		if strings.EqualFold(strings.TrimSpace(key), "bucket") {
+			return true
+		}
+	}
+	return false
+}

cmd/mo-tool/main_test.go

@@ -0,0 +1,69 @@
+// Copyright 2026 Matrix Origin
+//
+// Licensed 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 main
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestNormalizeLegacyRemoteS3Args(t *testing.T) {
+	remoteArgs := "bucket=test,endpoint=http://minio:9000,region=us-east-1,key-prefix=data,key-id=id,key-secret=secret"
+	tests := []struct {
+		name string
+		args []string
+		want []string
+	}{
+		{
+			name: "separate legacy argument",
+			args: []string{"ckp", "list", "--backend=S3", "--s3", remoteArgs},
+			want: []string{"ckp", "list", "--backend=S3", "--remote-s3", remoteArgs},
+		},
+		{
+			name: "joined legacy argument",
+			args: []string{"ckp", "dump", "--s3=" + remoteArgs},
+			want: []string{"ckp", "dump", "--remote-s3=" + remoteArgs},
+		},
+		{
+			name: "bucket with default AWS settings",
+			args: []string{"ckp", "list", "--s3", "bucket=test"},
+			want: []string{"ckp", "list", "--remote-s3", "bucket=test"},
+		},
+		{
+			name: "local S3FS selector",
+			args: []string{"ckp", "list", "--s3", "/tmp/mo-data"},
+			want: []string{"ckp", "list", "--s3", "/tmp/mo-data"},
+		},
+		{
+			name: "current remote argument",
+			args: []string{"ckp", "list", "--remote-s3", remoteArgs},
+			want: []string{"ckp", "list", "--remote-s3", remoteArgs},
+		},
+		{
+			name: "other command",
+			args: []string{"object", "stat", "--s3", remoteArgs},
+			want: []string{"object", "stat", "--s3", remoteArgs},
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			original := append([]string(nil), test.args...)
+			require.Equal(t, test.want, normalizeLegacyRemoteS3Args(test.args))
+			require.Equal(t, original, test.args)
+		})
+	}
+}

pkg/bootstrap/versions/v4_0_0/upgrade_test.go

@@ -22,6 +22,7 @@ import (
 	"github.com/golang/mock/gomock"
 	"github.com/prashantv/gostub"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 
 	"github.com/matrixorigin/matrixone/pkg/bootstrap/versions"
 	"github.com/matrixorigin/matrixone/pkg/catalog"
@@ -48,6 +49,10 @@ func mockTenantUpgrade(t *testing.T) {
 }
 
 func Test_Upgrade(t *testing.T) {
+	originalTenantUpgEntries := append([]versions.UpgradeEntry(nil), tenantUpgEntries...)
+	defer func() {
+		tenantUpgEntries = originalTenantUpgEntries
+	}()
 	mockTenantUpgrade(t)
 
 	sid := ""
@@ -85,11 +90,11 @@ func Test_Upgrade(t *testing.T) {
 }
 
 func Test_versionHandle_HandleClusterUpgrade(t *testing.T) {
-	originalEntries := clusterUpgEntries
-	clusterUpgEntries = nil
+	originalClusterUpgEntries := append([]versions.UpgradeEntry(nil), clusterUpgEntries...)
 	defer func() {
-		clusterUpgEntries = originalEntries
+		clusterUpgEntries = originalClusterUpgEntries
 	}()
+	clusterUpgEntries = []versions.UpgradeEntry{}
 
 	v := &versionHandle{
 		metadata: versions.Version{
@@ -109,6 +114,16 @@ func Test_versionHandle_HandleClusterUpgrade(t *testing.T) {
 	assert.Nil(t, err)
 }
 
+func TestUpgradeDefersMoIndexesIncludeColumn(t *testing.T) {
+	for _, entries := range [][]versions.UpgradeEntry{clusterUpgEntries, tenantUpgEntries} {
+		for _, entry := range entries {
+			require.False(t,
+				entry.Schema == catalog.MO_CATALOG && entry.TableName == catalog.MO_INDEXES && entry.UpgType == versions.ADD_COLUMN,
+				"mo_indexes schema changes require a staged release after all writers use explicit column lists")
+		}
+	}
+}
+
 func Test_upg_create_mo_task_sql_task_check_exists(t *testing.T) {
 	stubs := gostub.Stub(&versions.CheckTableDefinition, func(txn executor.TxnExecutor, accountId uint32, schema string, table string) (bool, error) {
 		assert.Equal(t, uint32(0), accountId)

pkg/bootstrap/versions/v4_0_1/upgrade_test.go

@@ -21,8 +21,10 @@ import (
 	"github.com/golang/mock/gomock"
 	"github.com/prashantv/gostub"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 
 	"github.com/matrixorigin/matrixone/pkg/bootstrap/versions"
+	"github.com/matrixorigin/matrixone/pkg/catalog"
 	"github.com/matrixorigin/matrixone/pkg/common/moerr"
 	"github.com/matrixorigin/matrixone/pkg/common/runtime"
 	mock_frontend "github.com/matrixorigin/matrixone/pkg/frontend/test"
@@ -94,6 +96,16 @@ func Test_versionHandle_HandleClusterUpgrade(t *testing.T) {
 	assert.Nil(t, err)
 }
 
+func TestUpgradeDefersMoIndexesIncludeColumn(t *testing.T) {
+	for _, entries := range [][]versions.UpgradeEntry{clusterUpgEntries, tenantUpgEntries} {
+		for _, entry := range entries {
+			require.False(t,
+				entry.Schema == catalog.MO_CATALOG && entry.TableName == catalog.MO_INDEXES && entry.UpgType == versions.ADD_COLUMN,
+				"mo_indexes schema changes require a staged release after all writers use explicit column lists")
+		}
+	}
+}
+
 func Test_upg_statistics_view_check_error(t *testing.T) {
 	stubs := gostub.Stub(&versions.CheckViewDefinition, func(txn executor.TxnExecutor, accountId uint32, schema string, viewName string) (bool, string, error) {
 		return true, "", moerr.NewInternalErrorNoCtx("return error")

pkg/catalog/secondary_index_utils.go

@@ -113,7 +113,10 @@ const (
 	IntermediateGraphDegree = "intermediate_graph_degree"
 	GraphDegree             = "graph_degree"
 	ITopkSize               = "itopk_size"
-	IncludedColumns         = "included_columns"
+	// IncludedColumns persists INCLUDE metadata inside algo_params. Consumers
+	// prefer plan.IndexDef.IncludedColumns when present and fall back to this key
+	// for catalog-loaded definitions.
+	IncludedColumns = "included_columns"
 
 	// Index-defining build params, settable as CREATE INDEX options (parsed by
 	// each plugin's ParamsFromTree). Written into flat algo_params only when
@@ -127,6 +130,49 @@ const (
 	IndexAlgoParamPrefixLengths = "prefix_lengths"
 )
 
+func ParseIncludeColumnsValue(raw string) ([]string, error) {
+	raw = strings.TrimSpace(raw)
+	if raw == "" {
+		return nil, nil
+	}
+
+	if strings.HasPrefix(raw, "[") {
+		var cols []string
+		if err := json.Unmarshal([]byte(raw), &cols); err == nil {
+			return cols, nil
+		}
+	}
+
+	parts := strings.Split(raw, ",")
+	cols := make([]string, 0, len(parts))
+	for _, part := range parts {
+		part = strings.TrimSpace(part)
+		if part == "" {
+			continue
+		}
+		cols = append(cols, part)
+	}
+	if len(cols) == 0 {
+		return nil, nil
+	}
+	return cols, nil
+}
+
+// MarshalIncludeColumnsValue encodes INCLUDE column names without losing
+// commas or leading/trailing whitespace that are valid inside quoted SQL
+// identifiers. ParseIncludeColumnsValue retains support for the historical
+// comma-separated representation when catalog metadata is reloaded.
+func MarshalIncludeColumnsValue(cols []string) (string, error) {
+	if len(cols) == 0 {
+		return "", nil
+	}
+	encoded, err := json.Marshal(cols)
+	if err != nil {
+		return "", err
+	}
+	return string(encoded), nil
+}
+
 /* 1. ToString Functions */
 
 // IndexParamsToStringList used by buildShowCreateTable and restoreDDL
@@ -223,19 +269,6 @@ func IndexParamsToStringList(indexParams string) (string, error) {
 	if val, ok := result[IndexAlgoParamQuantizerTrainLimit]; ok {
 		res += fmt.Sprintf(" %s = %s ", IndexAlgoParamQuantizerTrainLimit, val)
 	}
-
-	if val, ok := result[IncludedColumns]; ok && len(val) > 0 {
-		raw := strings.Split(val, ",")
-		parts := make([]string, 0, len(raw))
-		for _, p := range raw {
-			if p = strings.TrimSpace(p); p != "" {
-				parts = append(parts, p)
-			}
-		}
-		if len(parts) > 0 {
-			res += " INCLUDE (" + strings.Join(parts, ", ") + ") "
-		}
-	}
 	return res, nil
 }
 
@@ -425,7 +458,7 @@ func indexParamsToMap(def interface{}) (map[string]string, error) {
 		case tree.INDEX_TYPE_BTREE, tree.INDEX_TYPE_INVALID, tree.INDEX_TYPE_RTREE:
 			// do nothing
 		case tree.INDEX_TYPE_MASTER:
-			// do nothing
+		// do nothing
 		default:
 			// Vector algorithms (IVFFLAT / HNSW / CAGRA / IVFPQ) build their
 			// algo_params via the per-plugin plan hook BuildIndexParams; they

pkg/catalog/secondary_index_utils_test.go

@@ -59,7 +59,6 @@ func newPrefixKeyPart(colName string, length int) *tree.KeyPart {
 }
 
 func TestIsIndexAsync(t *testing.T) {
-
 	var (
 		json string
 		err  error
@@ -252,3 +251,56 @@ func TestIndexPrefixLengthsFromParams(t *testing.T) {
 	require.Nil(t, IndexPrefixLengthsFromParams("{bad json"))
 	require.Nil(t, IndexPrefixLengthsFromParams(`{"prefix_lengths":"t:0"}`))
 }
+
+func unresolvedName(name string) *tree.UnresolvedName {
+	return tree.NewUnresolvedName(tree.NewCStr(name, 1))
+}
+
+func TestIndexParamsToJsonString_RejectsPluginIvfFlatPath(t *testing.T) {
+	idx := tree.NewIndex(
+		false,
+		[]*tree.KeyPart{tree.NewKeyPart(unresolvedName("embedding"), -1, tree.DefaultDirection, nil)},
+		"idx1",
+		tree.INDEX_TYPE_IVFFLAT,
+		&tree.IndexOption{
+			AlgoParamList:         10,
+			AlgoParamVectorOpType: "vector_l2_ops",
+			IncludeColumns: []*tree.UnresolvedName{
+				unresolvedName("title"),
+				unresolvedName("category"),
+			},
+		},
+	)
+
+	params, err := IndexParamsToJsonString(idx)
+	require.Error(t, err)
+	require.Empty(t, params)
+	require.Contains(t, err.Error(), "invalid index alogorithm type")
+}
+
+func TestIndexParamsToStringList_DoesNotRenderIncludeColumns(t *testing.T) {
+	paramList, err := IndexParamsToStringList(`{"lists":"10","op_type":"vector_l2_ops","included_columns":"title,category"}`)
+	require.NoError(t, err)
+	require.Contains(t, paramList, "lists = 10")
+	require.Contains(t, paramList, "op_type 'vector_l2_ops'")
+	require.NotContains(t, paramList, "INCLUDE")
+	require.NotContains(t, paramList, "title")
+}
+
+func TestParseIncludeColumnsValue_BackwardCompatible(t *testing.T) {
+	encoded, err := MarshalIncludeColumnsValue([]string{"title, label", " rank "})
+	require.NoError(t, err)
+	require.Equal(t, `["title, label"," rank "]`, encoded)
+
+	cols, err := ParseIncludeColumnsValue(encoded)
+	require.NoError(t, err)
+	require.Equal(t, []string{"title, label", " rank "}, cols)
+
+	cols, err = ParseIncludeColumnsValue("title,category")
+	require.NoError(t, err)
+	require.Equal(t, []string{"title", "category"}, cols)
+
+	cols, err = ParseIncludeColumnsValue("[legacy,other")
+	require.NoError(t, err)
+	require.Equal(t, []string{"[legacy", "other"}, cols)
+}