You are viewing a plain text version of this content. The canonical link for it is here.

Posted to notifications@shardingsphere.apache.org by "schizophrenicmaniac (via GitHub)" <gi...@apache.org> on 2026/10/24 05:53:15 UTC

[PR] Fix Firebird describe of integer arithmetic expressions [shardingsphere]

schizophrenicmaniac opened a new pull request, #40361:
URL: https://github.com/apache/shardingsphere/pull/40361

Fixes #40356.

Changes proposed in this pull request:
- Describe `+`, `-`, `*` and `/` projections as BIGINT named ADD, SUBTRACT, MULTIPLY and DIVIDE, matching what Firebird returns in dialect 3. Before, they were described as INTEGER named after the operator while the backend sent 8-byte values, so the client misread the row data and closed the connection.
- Update the binary expression test and add one covering the remaining operators, a nested expression and an alias.

---

Before submitting this PR, I confirm that:
- [x] My code follows the [code of conduct](https://shardingsphere.apache.org/community/en/involved/conduct/code/) of this project.
- [x] I have reviewed my changes.
- [ ] I have (or in comment I request) added corresponding labels for the pull request.
- [x] I have passed the required local checks: `./mvnw clean install -B -T1C -Pcheck`.
- [ ] I have updated the relevant documentation, if applicable.
- [x] I have added or updated unit tests for my changes, if applicable.
- [ ] I have updated the Release Notes of the current development version, if applicable. For more details, see [Update Release Note](https://shardingsphere.apache.org/community/en/involved/contribute/contributor/).

AI disclosure: drafted with Claude Code (Claude Opus 5.5); files: FirebirdPrepareStatementCommandExecutor.java, FirebirdPrepareStatementCommandExecutorTest.java.

--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Fix Firebird describe of integer arithmetic expressions [shardingsphere]

Posted by "schizophrenicmaniac (via GitHub)" <gi...@apache.org>.

schizophrenicmaniac commented on PR #40361:
URL: https://github.com/apache/shardingsphere/pull/40361#issuecomment-6450650228

   @terrymanu Thanks, all four are fixed in the latest commit.
   
   CURRENT_DATE, LOCALTIME and LOCALTIMESTAMP are now typed as DATE, TIME and TIMESTAMP in getFunctionType, so they're resolved correctly as operands. CURRENT_DATE - CURRENT_DATE and end_date - CURRENT_DATE are described as INTEGER (4 bytes), and end_date - LOCALTIMESTAMP as NUMERIC. A plain SELECT CURRENT_DATE is now described as DATE too, which matches what Firebird returns. I left CURRENT_TIME and CURRENT_TIMESTAMP as they were, since they return the WITH TIME ZONE types on Firebird 4+.
   
   When both operands are NULL literals, the expression is described as INTEGER again. I removed the ordinary comment and annotated getLiteralType's parameter with @Nullable.
   
   I added CURRENT_DATE - CURRENT_DATE, end_date - CURRENT_DATE, end_date - LOCALTIMESTAMP and NULL + NULL to the parameterized test, which checks the described type and length. I didn't add a row-encoding assertion there because the cells are encoded from the backend result metadata outside this executor, so that check would belong in another class's test.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Fix Firebird describe of integer arithmetic expressions [shardingsphere]

Posted by "schizophrenicmaniac (via GitHub)" <gi...@apache.org>.

schizophrenicmaniac commented on PR #40361:
URL: https://github.com/apache/shardingsphere/pull/40361#issuecomment-6384035833

   @terrymanu Thanks, you're right about DATE - DATE. The arithmetic type is now worked out from the operand types following the dialect 3 rules: DATE - DATE is INTEGER, a date/time plus or minus a number keeps the date/time type, other date/time differences are NUMERIC, FLOAT/REAL/DOUBLE operands give DOUBLE, and only exact numerics give BIGINT. Operands are typed from table metadata for columns and from the value for literals, and nested expressions are resolved recursively. Operands that can't be typed, like parameters, still fall back to BIGINT.
   
   About the row data side: the fetched cells are encoded from the backend's result set metadata, not from this describe, so the describe only stays consistent if it reports the same type Firebird returns. The new parameterized test in FirebirdPrepareStatementCommandExecutorTest checks the described type and length for integer arithmetic, DATE - DATE (INTEGER, 4 bytes), a nested date expression, date/time plus a number, timestamp minus date and a DOUBLE operand.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Fix Firebird describe of integer arithmetic expressions [shardingsphere]

Posted by "schizophrenicmaniac (via GitHub) via notifications" <no...@shardingsphere.apache.org>.

schizophrenicmaniac commented on PR #40361:
URL: https://github.com/apache/shardingsphere/pull/40361#issuecomment-6772501316

@terrymanu Thanks, all four are addressed in the latest commit, and I've also handled the fractional precision point from your follow-up review.

1. A date or time plus a TIME WITH TIME ZONE or TIMESTAMP WITH TIME ZONE operand is now described as TIMESTAMP WITH TIME ZONE. CURRENT_TIME and CURRENT_TIMESTAMP are typed as their WITH TIME ZONE types when they're arithmetic operands, since that's what Firebird 4+ returns, so CURRENT_TIMESTAMP + 1 keeps the time zone too.

2. Exact multiplication and division now promote to INT128 when either side is BIGINT or INT128. Addition and subtraction stay BIGINT unless an operand is INT128. So (INTEGER + 1) * 2 is INT128, and SUM/AVG follow the same widening (INTEGER gives BIGINT, BIGINT gives INT128). On the descriptor itself: Jaybird reports INT128 results to the proxy as NUMERIC, and FirebirdBinaryRowBuilder writes NUMERIC cells as 8 bytes, which is how INT128 table columns already go through. Describing these results as SQL_INT128 (16 bytes) would desynchronize the row data, so they're described as NUMERIC with scale 0 instead. Sending real INT128 cells would need a change to the row encoding that also affects INT128 columns, and I'd rather do that separately.

3. The system table case now reads RDB$RELATION_ID of the RDB$DATABASE row from RDB$RELATIONS. System relation IDs are fixed, so the value is the same in every fixture database.

4. Renamed to expectedColumnType.

On the fractional results: the describe always sends scale 0, and the row encoder writes NUMERIC cells with longValue(), so the fraction is dropped for plain NUMERIC columns as well. Getting this right needs the scale carried through both the describe and the row encoding. For now, exact arithmetic with a NUMERIC/DECIMAL operand (columns, fractional literals, CASTs) keeps the INTEGER describe from master instead of claiming scale 0. The same goes for date/time differences other than DATE - DATE, which Firebird returns as NUMERIC(18,9) or NUMERIC(9,4). DOUBLE operands still give DOUBLE, and a date/time plus a NUMERIC still keeps its type.

The parameterized test now checks the sub type as well, so NUMERIC and BIGINT can be told apart. It covers the time zone sums, nested multiplication and division, BIGINT and SUM promotion, and the fractional fallback.

--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Fix Firebird describe of integer arithmetic expressions [shardingsphere]

Posted by "schizophrenicmaniac (via GitHub) via notifications" <no...@shardingsphere.apache.org>.

schizophrenicmaniac commented on PR #40361:
URL: https://github.com/apache/shardingsphere/pull/40361#issuecomment-6597312388

@terrymanu Thanks, all four are addressed in the latest commit.

1. Unknown operand types are now kept apart from real INTEGER and carried up through nested expressions. That covers CASE, unrecognised functions, columns that can't be resolved, non-numeric operands and CASTs to types it doesn't handle. If any operand is unknown, the whole arithmetic expression falls back to the INTEGER describe from master, so (CASE ... END + 1) - start_date is INTEGER again rather than DATE. IIF is now typed from its two result arguments, so subtracting two DATE-valued IIFs is INTEGER (4 bytes).

2. COALESCE now takes the common type of all its arguments, so COALESCE(1, CAST(2 AS DOUBLE PRECISION)) + 1 is DOUBLE. NULLIF takes the type of its first argument, following its CASE WHEN a = b THEN NULL ELSE a END definition. Mixed DATE/TIMESTAMP gives TIMESTAMP, and any other incompatible mix falls back like an unknown type.

3. The new E2E cases now use uppercase aliases, so the labels should match direct Firebird.

4. getFirstTypedParameterType is gone. The new getCommonType helper takes params.

I added the IIF date subtraction, nested CASE, and mixed-numeric COALESCE, NULLIF and IIF cases to the parameterized describe test.

--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Fix Firebird describe of integer arithmetic expressions [shardingsphere]

Posted by "schizophrenicmaniac (via GitHub)" <gi...@apache.org>.

schizophrenicmaniac commented on PR #40361:
URL: https://github.com/apache/shardingsphere/pull/40361#issuecomment-6461096352

   @terrymanu Thanks. CAST operands now take their target type in arithmetic, so CAST(... AS DATE) - CAST(... AS DATE) is described as INTEGER (4 bytes), a CAST timestamp minus a date as NUMERIC, and FLOAT/REAL/DOUBLE PRECISION casts as DOUBLE. Other target types keep the previous behaviour. I added those three cases to the parameterized describe test. I also moved getLiteralType below the arithmetic helpers so the declarations follow the call order in processExpressionProjection.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Fix Firebird describe of integer arithmetic expressions [shardingsphere]

Posted by "schizophrenicmaniac (via GitHub)" <gi...@apache.org>.

schizophrenicmaniac commented on PR #40361:
URL: https://github.com/apache/shardingsphere/pull/40361#issuecomment-6475475918

@terrymanu Thanks for the detailed review.

1. COALESCE and NULLIF operands now take the type of their first typed argument. A scalar subquery takes the type of its single projection (a column, an expression, or MAX/MIN of a column). So subtracting DATE-valued ones is described as INTEGER (4 bytes). CASE works differently: the Firebird parser doesn't build an expression tree for it yet and keeps it as a CommonExpressionSegment, so the executor can't see its branches. When an arithmetic operand is one of those, I kept the INTEGER describe from master instead of guessing, so CASE ... END - start_date is INTEGER as on the base.

2. Unbound columns in RETURNING are now resolved against the DML table. end_date - start_date and end_date - CURRENT_DATE in INSERT, UPDATE and DELETE ... RETURNING are described as INTEGER, and end_date + 1 as DATE.

3. A NULL literal now takes the other operand's type before the result type is worked out. So CURRENT_DATE - NULL, a DATE column minus NULL and the reversed forms are INTEGER. NULL + NULL stays INTEGER.

4. I added Firebird cases for the db scenario to e2e-dql-select.xml. They run through the Proxy and compare column labels, names and values with direct Firebird. They cover integer arithmetic over multiple rows including a NULL cell, a nested expression with aliases, DATE arithmetic with COALESCE and a NULL operand, and arithmetic on RDB$DATABASE. I haven't been able to run the Firebird E2E job locally, so they still need a run against a live server.

The parameterized describe test also covers all of the above, including the three RETURNING forms.

--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Fix Firebird describe of integer arithmetic expressions [shardingsphere]

Posted by "schizophrenicmaniac (via GitHub)" <gi...@apache.org>.

schizophrenicmaniac commented on PR #40361:
URL: https://github.com/apache/shardingsphere/pull/40361#issuecomment-6471835551

   @terrymanu On the E2E point, I'd like to keep this PR to the unit tests in FirebirdPrepareStatementCommandExecutorTest, which cover the arithmetic, alias, system-table and multi-row cases. I'll add Firebird cases to the Docker-based SQL E2E suite in a follow-up PR.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org