Skip to content

Commit f0b3f18

Browse files
authored
fix(iceberg): resilient namespace existence check (GET over HEAD) + CREATE NAMESPACE IF NOT EXISTS (#65)
* fix(iceberg): use GET instead of HEAD for namespace existence Some Iceberg REST servers (older apache/iceberg-rest builds on JdbcCatalog) do not implement HEAD /v1/namespaces/{ns} and reject it with 400 for any namespace, which made `up` die before the first migration. Probe existence via GET (LoadNamespaceProperties) instead: 404 -> not exists, success -> exists. * feat(iceberg): support CREATE NAMESPACE IF NOT EXISTS Parse the optional IF NOT EXISTS clause for CREATE NAMESPACE and record it in the IR. On dispatch, skip creation when the namespace already exists (checked via the GET-based NamespaceExists), making namespace creation idempotent. * chore: track CHANGELOG.md and add v1.8.2 notes
1 parent 24c9b91 commit f0b3f18

9 files changed

Lines changed: 275 additions & 21 deletions

File tree

.gitignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
!.goreleaser.yaml
66
*.md
77
!README.md
8+
!CHANGELOG.md
89
build.sh
910
docs
1011
dist

CHANGELOG.md

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
# Changelog
2+
3+
## v1.8.2
4+
5+
### Fixes
6+
- **iceberg**: use GET instead of HEAD for namespace existence (some REST servers reject HEAD with 400).
7+
8+
### Improvements
9+
- **iceberg**: support `CREATE NAMESPACE IF NOT EXISTS` (idempotent namespace creation).
10+
11+
## v1.6.0
12+
13+
### New Commands
14+
15+
#### `release` - Atomic Batch Apply
16+
Applies ALL pending migrations atomically within a single database transaction.
17+
18+
- All migrations in a release share the same `apply_time` value, enabling batch identification for later rollback
19+
- If any migration fails, the entire batch is rolled back automatically
20+
- Individual `.safe` migrations skip their inner transaction when inside a release (outer transaction provides atomicity)
21+
22+
```bash
23+
DSN="postgres://user:pass@localhost:5432/db" db-migrator release
24+
```
25+
26+
#### `rollback` - Atomic Batch Revert
27+
Reverts all migrations from the latest release batch, identified by `MAX(apply_time)`.
28+
29+
- Pre-checks that all `.down.sql` files exist before starting the rollback
30+
- Wraps all reverts in a single transaction for atomicity
31+
- If no release batch is found, shows an informational message
32+
33+
```bash
34+
DSN="postgres://user:pass@localhost:5432/db" db-migrator rollback
35+
```
36+
37+
### New Repository Methods
38+
- `InsertMigrationWithApplyTime` - insert migration record with explicit apply time (used by `release` to assign shared batch timestamp)
39+
- `MigrationsByMaxApplyTime` - query migrations belonging to the latest release batch
40+
41+
Implemented for all 4 database drivers: PostgreSQL, MySQL, ClickHouse, Tarantool.
42+
43+
### New Domain Service Methods
44+
- `ApplyFileWithApplyTime` - apply migration file with explicit apply time (reuses `applyFileCore` extracted from `ApplyFile`)
45+
- `LatestReleaseMigrations` - retrieve and map latest release batch migrations
46+
- `ExecInTransaction` - execute a function within a database transaction
47+
- `FileExists` - check whether a migration file exists
48+
49+
### Internal Improvements
50+
- Refactored `ApplyFile` into `applyFileCore` + wrapper for code reuse between `ApplyFile` and `ApplyFileWithApplyTime`
51+
- Added 23 new unit tests covering release handler, rollback handler, and new domain service methods
52+
- Updated documentation in CLAUDE.md and README.md
53+
54+
## v1.5.0
55+
56+
- Implementation of `to` command - bidirectional migration to specific version
57+
- Supports 4 version formats: timestamp, full name, datetime string, UNIX timestamp
58+
59+
## v1.4.0
60+
61+
- Implementation of dry run mode (`DRY_RUN=true`)
62+
63+
## v1.3.0
64+
65+
- Refactor to Clean Architecture
66+
- Credential masking in log output
67+
- SQL identifier validation
68+
- Comprehensive unit test coverage
69+
70+
## v1.2.0
71+
72+
- Tarantool database driver support
73+
74+
## v1.1.0
75+
76+
- ClickHouse cluster and replication support
77+
- MySQL driver support
78+
79+
## v1.0.0
80+
81+
- Initial release
82+
- PostgreSQL and ClickHouse support
83+
- Migration file management (up, down, redo, create, history, new)

internal/infrastructure/dal/repository/iceberg.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,15 @@ func (i *Iceberg) ExecQuery(ctx context.Context, query string, _ ...any) error {
199199

200200
switch op.Kind {
201201
case ddl.CreateNamespace:
202+
if op.IfNotExists {
203+
exists, err := i.cat.NamespaceExists(ctx, op.Table.Namespace)
204+
if err != nil {
205+
return err
206+
}
207+
if exists {
208+
return nil
209+
}
210+
}
202211
return i.cat.CreateNamespace(ctx, op.Table.Namespace, op.Props)
203212
case ddl.DropNamespace:
204213
return i.cat.DropNamespace(ctx, op.Table.Namespace)

internal/infrastructure/dal/repository/iceberg_test.go

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -764,6 +764,51 @@ func TestIceberg_ExecQuery_Dispatch(t *testing.T) {
764764
}
765765
}
766766

767+
// TestIceberg_ExecQuery_CreateNamespaceIfNotExists verifies idempotent namespace creation:
768+
// CREATE NAMESPACE IF NOT EXISTS skips CreateNamespace when the namespace already exists (checked
769+
// via the GET-based NamespaceExists), and calls it exactly once when it does not.
770+
func TestIceberg_ExecQuery_CreateNamespaceIfNotExists(t *testing.T) {
771+
ctx := context.Background()
772+
773+
t.Run("namespace exists → CreateNamespace skipped", func(t *testing.T) {
774+
repo, cat := newIcebergRepo(t)
775+
cat.EXPECT().Warehouse().Return("").Once()
776+
cat.EXPECT().
777+
NamespaceExists(ctx, []string{"analytics"}).
778+
Return(true, nil).Once()
779+
// No CreateNamespace expectation: the mock fails if it is called.
780+
781+
err := repo.ExecQuery(ctx, "CREATE NAMESPACE IF NOT EXISTS analytics")
782+
require.NoError(t, err)
783+
})
784+
785+
t.Run("namespace missing → CreateNamespace called once", func(t *testing.T) {
786+
repo, cat := newIcebergRepo(t)
787+
cat.EXPECT().Warehouse().Return("").Once()
788+
cat.EXPECT().
789+
NamespaceExists(ctx, []string{"analytics"}).
790+
Return(false, nil).Once()
791+
cat.EXPECT().
792+
CreateNamespace(ctx, []string{"analytics"}, (map[string]string)(nil)).
793+
Return(nil).Once()
794+
795+
err := repo.ExecQuery(ctx, "CREATE NAMESPACE IF NOT EXISTS analytics")
796+
require.NoError(t, err)
797+
})
798+
799+
t.Run("NamespaceExists error is propagated", func(t *testing.T) {
800+
repo, cat := newIcebergRepo(t)
801+
cat.EXPECT().Warehouse().Return("").Once()
802+
cat.EXPECT().
803+
NamespaceExists(ctx, []string{"analytics"}).
804+
Return(false, errors.New("catalog unreachable")).Once()
805+
806+
err := repo.ExecQuery(ctx, "CREATE NAMESPACE IF NOT EXISTS analytics")
807+
require.Error(t, err)
808+
assert.ErrorContains(t, err, "catalog unreachable")
809+
})
810+
}
811+
767812
// TestIceberg_ExecQuery_ParseError verifies that a parse error is propagated directly.
768813
func TestIceberg_ExecQuery_ParseError(t *testing.T) {
769814
ctx := context.Background()

internal/infrastructure/iceberg/catalog/catalog.go

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import (
2525
"strings"
2626

2727
iceberg "github.com/apache/iceberg-go"
28+
icebergcatalog "github.com/apache/iceberg-go/catalog"
2829
"github.com/apache/iceberg-go/catalog/rest"
2930
_ "github.com/apache/iceberg-go/io/gocloud" // register s3/gcs/azure schemes
3031
"github.com/pkg/errors"
@@ -129,11 +130,18 @@ func (c *Client) DropNamespace(ctx context.Context, ns []string) error {
129130

130131
// NamespaceExists checks whether the given namespace exists.
131132
func (c *Client) NamespaceExists(ctx context.Context, ns []string) (bool, error) {
132-
exists, err := c.cat.CheckNamespaceExists(ctx, ns)
133+
// Some Iceberg REST servers (older apache/iceberg-rest builds on JdbcCatalog) do not
134+
// implement HEAD /v1/namespaces/{ns} and reject it with 400 for any namespace. Probe via
135+
// GET (LoadNamespaceProperties) instead: 404 -> not found, success -> exists. GET is
136+
// supported by all spec-compliant REST catalogs.
137+
_, err := c.cat.LoadNamespaceProperties(ctx, ns)
133138
if err != nil {
139+
if errors.Is(err, icebergcatalog.ErrNoSuchNamespace) {
140+
return false, nil
141+
}
134142
return false, errors.WithMessage(err, "check namespace exists")
135143
}
136-
return exists, nil
144+
return true, nil
137145
}
138146

139147
// LoadNamespaceProperties returns the properties of the given namespace.

internal/infrastructure/iceberg/catalog/catalog_test.go

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,10 @@
99
package catalog_test
1010

1111
import (
12+
"context"
13+
"net/http"
14+
"net/http/httptest"
15+
"strings"
1216
"testing"
1317

1418
"github.com/raoptimus/db-migrator.go/internal/helper/dsn"
@@ -88,6 +92,72 @@ func TestNew_DSNParsing(t *testing.T) {
8892
}
8993
}
9094

95+
// brokenHeadCatalog emulates an Iceberg REST server (older apache/iceberg-rest builds on
96+
// JdbcCatalog) that does NOT implement HEAD /v1/namespaces/{ns} and rejects it with 400 for
97+
// any namespace, while GET works correctly. NamespaceExists must rely on GET only, so that a
98+
// missing namespace yields (false, nil) — not a bad-request error — even though HEAD returns 400.
99+
func brokenHeadCatalog(t *testing.T) *httptest.Server {
100+
t.Helper()
101+
return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
102+
w.Header().Set("Content-Type", "application/json")
103+
104+
// HEAD on any namespace is unsupported and rejected with 400.
105+
if r.Method == http.MethodHead && strings.HasPrefix(r.URL.Path, "/v1/namespaces/") {
106+
w.WriteHeader(http.StatusBadRequest)
107+
return
108+
}
109+
110+
switch r.URL.Path {
111+
case "/v1/config":
112+
w.WriteHeader(http.StatusOK)
113+
_, _ = w.Write([]byte(`{"defaults":{},"overrides":{}}`))
114+
case "/v1/namespaces/exists":
115+
w.WriteHeader(http.StatusOK)
116+
_, _ = w.Write([]byte(`{"namespace":["exists"],"properties":{}}`))
117+
case "/v1/namespaces/missing":
118+
w.WriteHeader(http.StatusNotFound)
119+
_, _ = w.Write([]byte(`{"error":{"message":"namespace does not exist",` +
120+
`"type":"NoSuchNamespaceException","code":404}}`))
121+
case "/v1/namespaces/boom":
122+
w.WriteHeader(http.StatusInternalServerError)
123+
_, _ = w.Write([]byte(`{"error":{"message":"boom","type":"ServerError","code":500}}`))
124+
default:
125+
w.WriteHeader(http.StatusNotFound)
126+
_, _ = w.Write([]byte(`{"error":{"message":"not found","type":"NotFound","code":404}}`))
127+
}
128+
}))
129+
}
130+
131+
// TestNamespaceExists_HeadUnsupported is a regression test for the dev bug where db-migrator up
132+
// died with "check namespace exists: REST error: bad request" against a REST server that rejects
133+
// HEAD /v1/namespaces/{ns} with 400. NamespaceExists must probe via GET, so it works despite HEAD
134+
// returning 400 — the passing test proves HEAD is no longer used.
135+
func TestNamespaceExists_HeadUnsupported(t *testing.T) {
136+
ts := brokenHeadCatalog(t)
137+
defer ts.Close()
138+
139+
parsed, err := dsn.Parse("iceberg://" + strings.TrimPrefix(ts.URL, "http://") + "/warehouse")
140+
require.NoError(t, err)
141+
142+
client, err := catalog.New(parsed)
143+
require.NoError(t, err)
144+
require.NotNil(t, client)
145+
146+
ctx := context.Background()
147+
148+
exists, err := client.NamespaceExists(ctx, []string{"exists"})
149+
require.NoError(t, err)
150+
assert.True(t, exists, "existing namespace must be reported as existing")
151+
152+
exists, err = client.NamespaceExists(ctx, []string{"missing"})
153+
require.NoError(t, err, "missing namespace must not surface HEAD 400 as an error")
154+
assert.False(t, exists, "missing namespace must be reported as not existing")
155+
156+
_, err = client.NamespaceExists(ctx, []string{"boom"})
157+
require.Error(t, err, "a non-404 error on GET must be propagated")
158+
assert.Contains(t, err.Error(), "check namespace exists")
159+
}
160+
91161
// TestNew_InvalidOAuth2ServerURI verifies that a malformed oauth2_server_uri returns an error.
92162
func TestNew_InvalidOAuth2ServerURI(t *testing.T) {
93163
parsed, err := dsn.Parse("iceberg://localhost:8181/warehouse?credential=c:s&oauth2_server_uri=://bad")

internal/infrastructure/iceberg/ddl/ir.go

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -27,14 +27,15 @@ type Ident struct {
2727

2828
// Operation is the IR produced by the parser for a single Spark-SQL DDL statement.
2929
type Operation struct {
30-
Kind OpKind
31-
Table Ident
32-
RenameTo *Ident // RenameTable: destination identifier
33-
Column *Field // AddColumn / DropColumn / AlterColumnType / RenameColumn (source column)
34-
NewName string // RenameColumn: new column name
35-
Partition *PartitionField // AddPartitionField / DropPartitionField
36-
Create *CreateTableSpec // CreateTable: full table specification
37-
Props map[string]string // CreateNamespace: optional properties
30+
Kind OpKind
31+
Table Ident
32+
RenameTo *Ident // RenameTable: destination identifier
33+
Column *Field // AddColumn / DropColumn / AlterColumnType / RenameColumn (source column)
34+
NewName string // RenameColumn: new column name
35+
Partition *PartitionField // AddPartitionField / DropPartitionField
36+
Create *CreateTableSpec // CreateTable: full table specification
37+
Props map[string]string // CreateNamespace: optional properties
38+
IfNotExists bool // CreateNamespace: skip if the namespace already exists
3839
}
3940

4041
// CreateTableSpec holds the full specification of a CREATE TABLE statement.

internal/infrastructure/iceberg/ddl/parse.go

Lines changed: 24 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -161,31 +161,44 @@ func (p *parser) parseCreate() (Operation, error) {
161161
}
162162
}
163163

164+
// consumeIfNotExists consumes an optional "IF NOT EXISTS" clause and reports whether it was present.
165+
func (p *parser) consumeIfNotExists() (bool, error) {
166+
if !p.peekUpperIs("IF") {
167+
return false, nil
168+
}
169+
p.consume()
170+
if err := p.expectConsume("NOT"); err != nil {
171+
return false, err
172+
}
173+
if err := p.expectConsume("EXISTS"); err != nil {
174+
return false, err
175+
}
176+
return true, nil
177+
}
178+
164179
func (p *parser) parseCreateNamespace() (Operation, error) {
165180
p.mustConsume(kwNAMESPACE)
181+
ifNotExists, err := p.consumeIfNotExists()
182+
if err != nil {
183+
return Operation{}, err
184+
}
166185
ns, err := p.parseNamespaceIdent()
167186
if err != nil {
168187
return Operation{}, err
169188
}
170189
op := Operation{
171-
Kind: CreateNamespace,
172-
Table: Ident{Namespace: ns},
190+
Kind: CreateNamespace,
191+
Table: Ident{Namespace: ns},
192+
IfNotExists: ifNotExists,
173193
}
174194
// Remaining tokens may be PROPERTIES (ignore for now, not in subset v1).
175195
return op, nil
176196
}
177197

178198
func (p *parser) parseCreateTable() (Operation, error) {
179199
p.mustConsume(kwTABLE)
180-
// Optional IF NOT EXISTS
181-
if p.peekUpperIs("IF") {
182-
p.consume()
183-
if err := p.expectConsume("NOT"); err != nil {
184-
return Operation{}, err
185-
}
186-
if err := p.expectConsume("EXISTS"); err != nil {
187-
return Operation{}, err
188-
}
200+
if _, err := p.consumeIfNotExists(); err != nil {
201+
return Operation{}, err
189202
}
190203
id, err := p.parseIdent()
191204
if err != nil {

internal/infrastructure/iceberg/ddl/parse_test.go

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -554,6 +554,30 @@ func TestParse_CreateNamespace_WithCatalog(t *testing.T) {
554554
})
555555
}
556556

557+
// TestParse_CreateNamespace_IfNotExists verifies that the optional IF NOT EXISTS clause is
558+
// parsed and recorded in the IR, enabling idempotent namespace creation.
559+
func TestParse_CreateNamespace_IfNotExists(t *testing.T) {
560+
t.Parallel()
561+
562+
t.Run("with IF NOT EXISTS", func(t *testing.T) {
563+
t.Parallel()
564+
op, err := Parse("iceberg", "CREATE NAMESPACE IF NOT EXISTS iceberg.raw")
565+
require.NoError(t, err)
566+
assert.Equal(t, CreateNamespace, op.Kind)
567+
assert.Equal(t, []string{"raw"}, op.Table.Namespace)
568+
assert.True(t, op.IfNotExists)
569+
})
570+
571+
t.Run("without IF NOT EXISTS", func(t *testing.T) {
572+
t.Parallel()
573+
op, err := Parse("", "CREATE NAMESPACE analytics")
574+
require.NoError(t, err)
575+
assert.Equal(t, CreateNamespace, op.Kind)
576+
assert.Equal(t, []string{"analytics"}, op.Table.Namespace)
577+
assert.False(t, op.IfNotExists)
578+
})
579+
}
580+
557581
// TestParse_NotNull_OutsideSubset verifies that NOT NULL (outside subset v1) returns ErrParse
558582
// and does not panic. Field.Required is not supported in subset v1.
559583
func TestParse_NotNull_OutsideSubset(t *testing.T) {

0 commit comments

Comments
 (0)