You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@iceberg.apache.org by ze...@apache.org on 2026/08/12 17:55:43 UTC
(iceberg-go) branch main updated: fix(sql): validate view identifiers at API boundaries (#1571)
This is an automated email from the ASF dual-hosted git repository.
zeroshade pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/iceberg-go.git
The following commit(s) were added to refs/heads/main by this push:
new 73222a326 fix(sql): validate view identifiers at API boundaries (#1571)
73222a326 is described below
commit 73222a326afbfad0b9f1379cda3cb7081d6d78a2
Author: Minh Vu <vu...@gmail.com>
AuthorDate: Wed Aug 12 19:55:38 2026 +0200
fix(sql): validate view identifiers at API boundaries (#1571)
* fix(sql): validate view identifiers at API boundaries
Signed-off-by: Minh Vu <vu...@gmail.com>
* test(sql): clarify invalid view identifier cases
Signed-off-by: Minh Vu <vu...@gmail.com>
* fix(sql): validate namespace identifiers for view listing
* fix(sql): preserve V0 view semantics during validation
* test(sql): cover invalid view listing namespaces
* style(sql): satisfy lint in view identifier test
* fix(sql): keep view identifier validation at API boundaries
* test(sql): cover nested view listing
* fix(sql): reject empty namespace creation
* fix(sql): validate dropped view identifiers
* fix(sql): allow dropping legacy invalid views
---------
Signed-off-by: Minh Vu <vu...@gmail.com>
---
catalog/sql/sql.go | 12 +++++
catalog/sql/sql_test.go | 122 ++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 134 insertions(+)
diff --git a/catalog/sql/sql.go b/catalog/sql/sql.go
index 37f3aa365..3726070d2 100644
--- a/catalog/sql/sql.go
+++ b/catalog/sql/sql.go
@@ -1589,6 +1589,10 @@ func (c *Catalog) CreateView(ctx context.Context, identifier table.Identifier, s
return errViewsUnsupportedOnV0
}
+ if err := catalog.ValidateViewIdentifier(identifier); err != nil {
+ return err
+ }
+
nsIdent := catalog.NamespaceFromIdent(identifier)
viewIdent := catalog.TableNameFromIdent(identifier)
ns, exists, err := c.resolveNamespaceKey(ctx, nsIdent)
@@ -1796,6 +1800,10 @@ func (c *Catalog) CheckViewExists(ctx context.Context, identifier table.Identifi
return false, nil
}
+ if err := catalog.ValidateViewIdentifier(identifier); err != nil {
+ return false, err
+ }
+
ns, err := c.namespaceKey(ctx, catalog.NamespaceFromIdent(identifier))
if err != nil {
return false, err
@@ -1822,6 +1830,10 @@ func (c *Catalog) LoadView(ctx context.Context, identifier table.Identifier) (vi
return nil, errViewsUnsupportedOnV0
}
+ if err := catalog.ValidateViewIdentifier(identifier); err != nil {
+ return nil, err
+ }
+
ns, err := c.namespaceKey(ctx, catalog.NamespaceFromIdent(identifier))
if err != nil {
return nil, err
diff --git a/catalog/sql/sql_test.go b/catalog/sql/sql_test.go
index 365eb7b7c..391404fd0 100644
--- a/catalog/sql/sql_test.go
+++ b/catalog/sql/sql_test.go
@@ -43,6 +43,7 @@ import (
"github.com/apache/iceberg-go/table"
"github.com/google/uuid"
"github.com/stretchr/testify/assert"
+ "github.com/stretchr/testify/require"
"github.com/stretchr/testify/suite"
"github.com/uptrace/bun/driver/sqliteshim"
)
@@ -1769,6 +1770,16 @@ func (s *SqliteCatalogTestSuite) TestCreateDuplicateNamespace() {
}
}
+func (s *SqliteCatalogTestSuite) TestCreateNamespaceRejectsEmptyIdentifier() {
+ ctx := context.Background()
+ for _, cat := range []*sqlcat.Catalog{s.getCatalogMemory(), s.getCatalogSqlite()} {
+ for _, namespace := range []table.Identifier{nil, {}} {
+ err := cat.CreateNamespace(ctx, namespace, nil)
+ s.ErrorIs(err, catalog.ErrNoSuchNamespace)
+ }
+ }
+}
+
func (s *SqliteCatalogTestSuite) TestCreateNamespaceSharingPrefix() {
tests := []struct {
cat *sqlcat.Catalog
@@ -2206,16 +2217,84 @@ func (s *SqliteCatalogTestSuite) TestCreateView() {
nsName := databaseName()
viewName := tableName()
s.Require().NoError(db.CreateNamespace(context.Background(), []string{nsName}, nil))
+ nestedNamespace := table.Identifier{nsName, "child"}
+ s.Require().NoError(db.CreateNamespace(context.Background(), nestedNamespace, nil))
viewSQL := "SELECT * FROM test_table"
schema := iceberg.NewSchema(1, iceberg.NestedField{
ID: 1, Name: "id", Type: iceberg.PrimitiveTypes.Int32, Required: true,
})
s.Require().NoError(db.CreateView(context.Background(), []string{nsName, viewName}, schema, viewSQL, nil))
+ s.Require().NoError(db.CreateView(context.Background(), append(nestedNamespace, viewName), schema, viewSQL, nil))
exists, err := db.CheckViewExists(context.Background(), []string{nsName, viewName})
s.Require().NoError(err)
s.True(exists)
+
+ foundNested := false
+ for identifier, err := range db.ListViews(context.Background(), nestedNamespace) {
+ s.Require().NoError(err)
+ s.Equal(append(nestedNamespace, viewName), identifier)
+ foundNested = true
+ }
+ s.True(foundNested)
+}
+
+func TestViewOperationsRejectInvalidIdentifiers(t *testing.T) {
+ invalid := []struct {
+ name string
+ identifier table.Identifier
+ }{
+ {name: "nil", identifier: nil},
+ {name: "empty", identifier: table.Identifier{}},
+ {name: "missing namespace", identifier: table.Identifier{"view"}},
+ {name: "empty name", identifier: table.Identifier{"ns", ""}},
+ {name: "dot name", identifier: table.Identifier{"ns", "."}},
+ {name: "parent name", identifier: table.Identifier{"ns", ".."}},
+ {name: "path separator", identifier: table.Identifier{"ns", "nested/view"}},
+ {name: "control character", identifier: table.Identifier{"ns", "view\nname"}},
+ }
+
+ loaded, err := catalog.Load(context.Background(), "default", iceberg.Properties{
+ "uri": ":memory:",
+ sqlcat.DriverKey: sqliteshim.ShimName,
+ sqlcat.DialectKey: string(sqlcat.SQLite),
+ sqlcat.SchemaVersionKey: sqlcat.SchemaVersionV1,
+ "type": "sql",
+ })
+ require.NoError(t, err)
+ cat := loaded.(*sqlcat.Catalog)
+ require.NoError(t, cat.CreateSQLTables(context.Background()))
+ for _, err := range cat.ListViews(context.Background(), nil) {
+ require.NoError(t, err, "ListViews with a nil namespace must not reject the root namespace")
+ }
+
+ for _, test := range invalid {
+ t.Run(test.name, func(t *testing.T) {
+ err := cat.CreateView(context.Background(), test.identifier, nil, "", nil)
+ assert.ErrorIs(t, err, catalog.ErrNoSuchView)
+
+ _, err = cat.CheckViewExists(context.Background(), test.identifier)
+ assert.ErrorIs(t, err, catalog.ErrNoSuchView)
+
+ _, err = cat.LoadView(context.Background(), test.identifier)
+ assert.ErrorIs(t, err, catalog.ErrNoSuchView)
+ })
+ }
+
+ t.Run("list views rejects missing namespace", func(t *testing.T) {
+ yielded := false
+ var gotErr error
+ for _, err := range cat.ListViews(context.Background(), table.Identifier{".."}) {
+ yielded = true
+ gotErr = err
+
+ break
+ }
+
+ require.True(t, yielded)
+ require.ErrorIs(t, gotErr, catalog.ErrNoSuchNamespace)
+ })
}
func (s *SqliteCatalogTestSuite) TestCreateTableConflictsWithView() {
@@ -2378,6 +2457,36 @@ func (s *SqliteCatalogTestSuite) TestDropView() {
s.ErrorIs(err, catalog.ErrNoSuchView)
}
+func (s *SqliteCatalogTestSuite) TestDropViewRemovesLegacyInvalidView() {
+ ctx := context.Background()
+ db := s.getCatalogSqlite()
+ s.Require().NoError(db.CreateSQLTables(ctx))
+
+ nsName := databaseName()
+ viewName := "nested/view"
+ s.Require().NoError(db.CreateNamespace(ctx, []string{nsName}, nil))
+
+ sqldb := s.getDB()
+ defer sqldb.Close()
+
+ _, err := sqldb.ExecContext(ctx,
+ `INSERT INTO iceberg_tables `+
+ `(catalog_name, table_namespace, table_name, iceberg_type) VALUES (?, ?, ?, ?)`,
+ db.Name(), nsName, viewName, sqlcat.ViewType,
+ )
+ s.Require().NoError(err)
+
+ s.Require().NoError(db.DropView(ctx, table.Identifier{nsName, viewName}))
+
+ var count int
+ s.Require().NoError(sqldb.QueryRowContext(ctx,
+ `SELECT COUNT(*) FROM iceberg_tables `+
+ `WHERE catalog_name = ? AND table_namespace = ? AND table_name = ? AND iceberg_type = ?`,
+ db.Name(), nsName, viewName, sqlcat.ViewType,
+ ).Scan(&count))
+ s.Zero(count)
+}
+
func (s *SqliteCatalogTestSuite) TestDropViewWithInvalidMetadataLocation() {
db := s.getCatalogSqlite()
s.Require().NoError(db.CreateSQLTables(context.Background()))
@@ -2463,12 +2572,25 @@ func (s *SqliteCatalogTestSuite) TestListViews() {
}
viewsIter = db.ListViews(context.Background(), []string{"nonexistent"})
+ yielded := false
for _, err := range viewsIter {
+ yielded = true
s.Error(err)
s.ErrorIs(err, catalog.ErrNoSuchNamespace)
break
}
+ s.Require().True(yielded)
+
+ viewsIter = db.ListViews(context.Background(), []string{".."})
+ yielded = false
+ for _, err := range viewsIter {
+ yielded = true
+ s.ErrorIs(err, catalog.ErrNoSuchNamespace)
+
+ break
+ }
+ s.Require().True(yielded)
}
func (s *SqliteCatalogTestSuite) TestLoadView() {