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

Issue on prepared statement if the parameter is null (e.g. optional parameter, research result of #579) #1365

Closed
speckyspooky opened this issue Jul 13, 2023 · 4 comments
Assignees
Labels
BugFix Change to correct issues
Milestone

Comments

@speckyspooky
Copy link
Contributor

I analysed the old PR #579.
It didn't understand what the target in full it seems to me to improve the exception message to get it more readable.

But I was interested on the main reason of the issue, so I veryfied the testing report at the communication and started a deep research.
And the reason of the exception is I think an "old"-topic/"known error". The reason is that the query cannot be executed if the BIRT-parameter of a query-parameter is null.
The most common way is here to use an optional parameter and set him to "null value" (then we will get the exception).

The technical cause of the exception is the implementation of the jdbc-method on the prepared statement to set the "null" value with "isNull()". The problem there is that the parameter type should be fetched to set the value but the current implementation won't be supported through Oracle with the ojdbc-driver:

relevant implementation line at class "Statement", method "setNull":
this.preStat.setNull(parameterId, pm.getParameterType(parameterId));

My suggestion to fix it would be to use the standard way which supports oracle in combination with the current version.

statement-setnull

I have done the tests in both versions of statements which was descripted on #579 and I tested it with Oracle V19 and MS SQL Server 2019. With the small change the exception will be avoid. Here my result screens:

statement fixed

I would prefer to create here the according PR to fix it.
Any hint to improve the change or avoid the "doubled try-catch" is welcome.

@speckyspooky speckyspooky self-assigned this Jul 13, 2023
@speckyspooky speckyspooky added this to the 4.14 milestone Jul 13, 2023
@hvbtup
Copy link
Contributor

hvbtup commented Jul 14, 2023

I'm too busy to test it, but to me it seems reasonable.
I remember when I debugged this back then, it seemed to me that BIRT wasn't able to detect the type correctly.
Which confused me, because it should be straightforward to determine the parameter type without magic type because the type is defined in the rptdesign. But then again, it's Javascript...

The idea to test it with an optional report parameter is good, however, please test it with a sub-report, too:

Say with we have a table PERSONS containing colums ID (as primary key), NAME, MOTHER_ID, FATHER_ID
(First test: The ID columns are VARCHAR2(10), second test: The ID columnd are NUMBER(10).
For most persons, MOTHER_ID and FATHER_ID are set.
For an orphan, MOTHER_ID and FATHER_ID are NULL.
For other persons, maybe only the FATHER_ID is NULL.

Now we want to create a report like this:

Outer report: DataSet persons
SELECT *
FROM PERSONS
ORDER BY NAME

Inner report: DataSet parents
WITH params AS (SELECT ? AS MOTHER_ID, ? AS FATHER_ID FROM DUAL)
SELECT persons.NAME,
CASE persons.ID
WHEN params.MOTHER_ID then 'Mom'
WHEN params.FATHER_ID then 'Dad'
END as relation
FROM params CROSS JOIN persons
WHERE persons.ID IN (params.MOTHER_ID, params.FATHER_ID)
ORDER BY relation

This DataSet needs two dataset parameters.

Let's use a Table Item for the persons dataset.
Add a new column.
Drop the parents dataset into the empty detail cell.
In the DataSet parameter dialog, use row["MOTHER_ID"] and row["FATHER_ID"] for the two dataset parameters.

Test if the report works correctly for orphans or if one of the two parents is null.

@speckyspooky
Copy link
Contributor Author

Ok, I think I understand your logic it is like a tree logic.
But It will work because the fix is directly before the JDBC-driver will be called. This level of prepared statement is the lowest which we can reach.

But nevertheless I will try to create an report according to your description.

@speckyspooky
Copy link
Contributor Author

I created an additional report with the loop tables based on string- and integer-id-columns.
The BIRT-loop works also with NULL-columns. This was working in BIRT-versions before because I figured out that here is the internal parameter definition of the prepared statement available and the original "setNull()"-method can be used.
(I researched it with the debugger,
be aware to avoid confusing: the parameter definition here is the definition of the SQL parameter meta data (JDBC/ODA) which is not the same object like BIRT-parameter-definition).

The good news the change will fixed such special kind of unaccessible NULL-column type
and the existing behavior is also given and will work.

So I will create a PR for it.

SQL_error_parameter_null_col_string-inner-loop.pdf

speckyspooky added a commit to speckyspooky/birt that referenced this issue Jul 15, 2023
@speckyspooky speckyspooky added the BugFix Change to correct issues label Jul 15, 2023
merks pushed a commit to speckyspooky/birt that referenced this issue Jul 15, 2023
speckyspooky added a commit that referenced this issue Jul 15, 2023
* Fix prepared statement setNull() #1365

* Fix missing comment (to restart the build process)
@speckyspooky
Copy link
Contributor Author

The issue is fixed with PR #1371

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

No branches or pull requests

2 participants