Skip to content

Fix AI-reported bugs against current main type rules - #8244

Open
mattcasters wants to merge 1 commit into
apache:mainfrom
mattcasters:issue-8218
Open

Fix AI-reported bugs against current main type rules#8244
mattcasters wants to merge 1 commit into
apache:mainfrom
mattcasters:issue-8218

Conversation

@mattcasters

Copy link
Copy Markdown
Contributor

Verified against current main / 2.20 type rules, not 2.19 getFieldDefinition. That is the answer to the review notes on #8220 / #8221.

Already handled by #8214 (Database perspective)

These are not in this PR. Comments are on the issues; they should close when #8214 merges:

#8217 was already closed (UNIQUEIDENTIFIER on both SQL Server dialects).

This PR

Issue Change
#8216 Hop Number length is JDBC precision (total digits). numericLength() stays integer digits for dialect rule conditions. PostgreSQL writes NUMERIC(length, scale) so it round-trips. Does not merge #8242 as-is (that inverts numericLength() and would inflate Postgres precision).
#8218 Timestamp parse mask reads HOP_DEFAULT_TIMESTAMP_FORMAT
#8219 Stream Lookup Timestamp default no longer throws ConversionNotImplemented
#8220 SQL Server DATE / TIME / DATETIME2 via type rules; new Date fields stay DATETIME
#8221 SQL Server NVARCHAR/NCHAR preserved; TEXT replaced with VARCHAR(MAX)
#8222 Parameter metadata goes through StandardJdbcTypeMapper (NVARCHAR, NUMERIC(p,s))
#8223 Ordinary JDBC no longer calls the data-service getColumns() path
#8225 Empty leftover metadata folders are not listed under Unknown
#8226 catalog.schema is split even when supportsCatalogs() is false
#8227 ProgressMonitorDialog disposes when the runnable returns (Get fields no longer hangs)
#8230 Dead View-menu terminal constants and unused USE_ADVANCED_TERMINAL key removed

Tests

Focused modules run under tools/with-isolated-display.sh: core, ui, mssql, mssqlnative, postgresql (+ redshift/greenplum/cockroach), mysql, oracle, streamlookup.

Fixes #8216 #8218 #8219 #8220 #8221 #8222 #8223 #8225 #8226 #8227 #8230

Verified against 2.20 DatabaseTypeMapper rather than 2.19 getFieldDefinition.

- apache#8216: store JDBC precision as Hop Number length; keep numericLength() as
  integer digits for dialect rules; PostgreSQL writes NUMERIC(length, scale)
- apache#8218: timestamp parse mask reads HOP_DEFAULT_TIMESTAMP_FORMAT
- apache#8219: Stream Lookup handles Timestamp default values
- apache#8220/apache#8221: SQL Server DATE/TIME/DATETIME2 and NVARCHAR/VARCHAR(MAX) via type rules
- apache#8222: parameter metadata uses StandardJdbcTypeMapper
- apache#8223: skip JDBC getColumns() for ordinary databases
- apache#8225: do not list empty unknown metadata folders
- apache#8226: split catalog.schema even when supportsCatalogs() is false
- apache#8227: ProgressMonitorDialog closes when the runnable returns
- apache#8230: remove dead terminal menu constants

Fixes apache#8216, apache#8218, apache#8219, apache#8220, apache#8221, apache#8222, apache#8223, apache#8225, apache#8226, apache#8227, apache#8230
@fskorgen

fskorgen commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Thanks for taking these on!

We run 2.19 in production and have local patches for a few of them, so we are not
blocked. But #8216 is the one that hurts: the shortened DECIMAL length breaks our
job runs. That fix looks self-contained in StandardJdbcTypeMapper, which is in 2.19
too, unlike the SQL Server ones that need the 2.20 type rules.

We have patched #8220/#8221 ourselves, but anyone else on 2.19 with SQL Server will
get wrong DDL for Unicode columns.

Is a 2.19 patch release planned, and could #8216 go into the 2.19 branch?

@mattcasters

Copy link
Copy Markdown
Contributor Author

Thanks for filing the issues @fskorgen. If you're not blocked my idea is that it's best for give this PR some mileage until the 2.20.0 release in October. 2.19.0 should have a compatibility flag for the DDL issues as well so not so blocking for others either I think.
The flag is gone in main (2.20.0) since the backend got reworked with even more flexibility. That's what the comments from @hansva were about.
If you find the time, do try out this PR and let us know if there's anything missing for your deployment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: DECIMAL read metadata is incompatible with DDL generation in 2.19 (regression)

2 participants