firebird icon indicating copy to clipboard operation
firebird copied to clipboard
verified publisher icon FirebirdSQL

Fix for #8082 by making engine to use user buffers directly

Open aafemt opened this issue 1 year ago • 7 comments

Plan "A" is simple:

  1. Get rid of EOS variable because user buffers have no room for it.
  2. Modify ParameterNode to use actual message metadata and message buffer instead of one generated during compilation and scratch area.

aafemt avatar May 30 '24 15:05 aafemt

At this point the testcase for #8082 produces exactly expected output.

aafemt avatar Jun 28 '24 16:06 aafemt

Modified testcase that also check coercion of input data. ctest.zip

aafemt avatar Jul 09 '24 15:07 aafemt

Previous commit should completely resolve #8185 moving linkage to parent cursor from DsqlStatement into DsqlRequest so changes from #8189 were partially undone.

aafemt avatar Jul 29 '24 16:07 aafemt

Previous commit should completely resolve #8185 moving linkage to parent cursor from DsqlStatement into DsqlRequest so changes from #8189 were partially undone.

#8185 is already fixed in different ways in master and v5. I see no relation of #8189 with this PR. I actually see no relation of this PR with actual problem of #8082.

In reality, I do not understand what is this PR about.

asfernandes avatar Jul 30 '24 00:07 asfernandes

What I see in master is just disabled caching for queries with cursor name set (which is 100% in Delphi applications). Reference to #8189 was mistype, I meant #8191. The change in this PR made DsqlStatement constant so it can be cached freely (up to reuse of single instance simultaneously).

aafemt avatar Jul 30 '24 09:07 aafemt

disabled caching for queries with cursor name set

Sorry, "reference to cursor" meant here. And Delphi applications are not affected more than others. My fault.

aafemt avatar Jul 30 '24 14:07 aafemt

Additionally failed QA tests:

  1. tests/bugs/core_4374_test.py failed because BLR generated for SUSPEND is three bytes shorter. (Redundant blr_begin, blr_stall, blr_end were removed.)
  2. tests/bugs/core_5973_test.py failed because deprecated SQLCODE was removed from error messages in refactored code.
  3. tests/bugs/gh_6910_test.py failed because output message is no more generated for Execute Block without output.
  4. tests/bugs/gh_7611_test.py failed because behavior of Batch matches behavior of BLR and the workaround doesn't work anymore.

aafemt avatar Aug 09 '24 16:08 aafemt

Could someone review this, please?

aafemt avatar Nov 08 '24 12:11 aafemt

Updated, ready to merge.

aafemt avatar Dec 05 '24 12:12 aafemt

I wonder why these Android runners are so unstable.

aafemt avatar Dec 05 '24 13:12 aafemt

Please cleanup req_user_descs that's no longer needed. I'm OK with the PR, just willing to do a few tests before merging.

dyemanov avatar Apr 29 '25 05:04 dyemanov

This branch crashes in tcs test FB_SQL_SUBPROC_1.

asfernandes avatar Apr 29 '25 10:04 asfernandes

This PR broke WHERE CURRENT OF maybe mixed with RETURNING. The tests I have is private, so I cannot easily share it, but probably any usage is going to cause problem. Error is: undefined message number

asfernandes avatar May 17 '25 01:05 asfernandes

Given the problems this PR seems to have introduced, shouldn't it be backed out/reverted?

mrotteveel avatar May 18 '25 12:05 mrotteveel

These problems are not big enough for that, IMHO. I'd prefer to fix them all one-by-one.

aafemt avatar May 18 '25 13:05 aafemt

This PR also breaks TCS test AUTO_COMMIT.3.ESQL.

asfernandes avatar Jun 26 '25 11:06 asfernandes

And it's also not fixed by https://github.com/FirebirdSQL/firebird/pull/8571.

asfernandes avatar Jun 26 '25 11:06 asfernandes