Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

aafemt
Copy link
Contributor

@aafemt aafemt commented May 30, 2024

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
Copy link
Contributor Author

aafemt commented Jun 28, 2024

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

@aafemt aafemt marked this pull request as ready for review July 9, 2024 15:31
@aafemt
Copy link
Contributor Author

aafemt commented Jul 9, 2024

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

@aafemt
Copy link
Contributor Author

aafemt commented Jul 29, 2024

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

@asfernandes
Copy link
Member

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.

@aafemt
Copy link
Contributor Author

aafemt commented Jul 30, 2024

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
Copy link
Contributor Author

aafemt commented Jul 30, 2024

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
Copy link
Contributor Author

aafemt commented Aug 9, 2024

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 aafemt changed the title Attempt to fix #8082 by making engine to use user buffers directly Fix for #8082 by making engine to use user buffers directly Aug 9, 2024
@aafemt aafemt marked this pull request as ready for review August 9, 2024 17:02
@aafemt
Copy link
Contributor Author

aafemt commented Nov 8, 2024

Could someone review this, please?

@dyemanov dyemanov self-requested a review November 8, 2024 12:20
@aafemt
Copy link
Contributor Author

aafemt commented Dec 5, 2024

Updated, ready to merge.

@aafemt
Copy link
Contributor Author

aafemt commented Dec 5, 2024

I wonder why these Android runners are so unstable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants