birt icon indicating copy to clipboard operation
birt copied to clipboard

JDBC ParameterMetaData - Feature not supported handling

Open tkrautinger opened this issue 4 years ago • 13 comments

In some situations, like an "complex" query cannot be parsed by the Oracle JDBC driver, the call of org.eclipse.birt.report.data.oda.jdbc.ParameterMetaData.getParameterType(...) throws an JDBCException.

SQL example:

SELECT 1 FROM DUAL WHERE 1 < ? AND SYSDATE > TO_DATE('20200101','YYYYMMDD')

The currently implemented MySQL handling (SQLExceptions with state "S1C00") has extened to fit also for Oracle feature not supported exceptions (SQLExceptions with code "17023") and SQLFeatureNotSupportedExceptions.

Signed-off-by: Thomas Krautinger

tkrautinger avatar Apr 21 '20 13:04 tkrautinger

@wimjongman Shouldn't we reopen this? I think this is a very useful contribution for all those who are using BIRT with Oracle.

hvbtup avatar Jan 05 '22 08:01 hvbtup

Yes, absolutely.

wimjongman avatar Jan 05 '22 11:01 wimjongman

I can test if this works (and reduces the flood of log messages) in the next few days.

hvbtup avatar Jan 05 '22 12:01 hvbtup

I can test if this works (and reduces the flood of log messages) in the next few days.

Thanks, Henning.

wimjongman avatar Jan 06 '22 13:01 wimjongman

I havent' found the time to test this yet. I'm still struggling with the flood of messages in my inbox which filled up last week when I was working as a BIRT coach. It's still on my to-do list.

hvbtup avatar Jan 18 '22 14:01 hvbtup

I was working as a BIRT coach.

Did you manage to recruit some new contributors? ;)

wimjongman avatar Jan 18 '22 16:01 wimjongman

I wanted to confirm the old behavior before testing the effect of the PR. Unfortunately I could not produce the huge stacktrace log of the old behavior when testing from inside the all-in-one IDE. It is certainly depending on the logging configuration. I did see the huge stacktrace in the log when using the BIRT POJO runtime, but the POJO runtime is not working at the moment, so I cannot test. So my test is blocked by https://github.com/eclipse/birt/discussions/738 / https://github.com/eclipse/birt/issues/781

hvbtup avatar Jan 19 '22 08:01 hvbtup

@tkrautinger can you please rebase your changes

@hvbtup the runtime issue is fixed so this can be tested now.

wimjongman avatar Jan 31 '22 14:01 wimjongman

With a very simple report based on Thomas query, I can create an issue in the New Preview Prototype, if the parameter value is null.

The following items have errors:

Table (id = 5512):

  • There is an error in the report query loading, Can not retrieve data to generate the report. Error.ReportQueryLoadingError ( 1 time(s) ) detail : org.eclipse.birt.report.engine.api.EngineException: There is an error in the report query loading, Can not retrieve data to generate the report. at org.eclipse.birt.report.engine.data.dte.DataPresentationEngine.doExecuteQuery(DataPresentationEngine.java:118)

With Run / View Report as / HTML (or PDF) I get the following error in the "Problems" view:

Failed to prepare the query execution for the data set: dual Cannot set a null value to parameter 1. org.eclipse.birt.report.data.oda.jdbc.JDBCException: Cannot set preparedStatement null value. SQL error #1:Unsupported feature: checkValidIndex

With a parameter value of 2, there is no error.

So I can confirm there is an issue (at least when such a query is called from the All-In-One-Designer).

Notes:

In the reports we are using in production (with BIRT 4.3.0), we are facing a similar issue, but not quite the same. In our reports, we often have null input for string parameters. If such a string input parameter is null, then we'll find huge strack traces in the debug output (if we enable it), but the report runs normally, because after logging the exception, BIRT uses a string for the unknown parameter type as a last resort.

This behaviour is still the same in the current master. I tested this by replacing the WHERE clause with WHERE 'X' = ? AND SYSDATE > TO_DATE('20200101','YYYYMMDD') and changing the parameter type from Integer to String.

In other words, with null values for string parameters, there is only an annoying message but otherwise everything is working as it should, whereas in the issue Thomas described here (with an Integer parameter), the report fails.

To avoid the annoying messages in the log, we use a function nvl_mt(x) { return (x==null ? "" : x); } to replace null parameters with empty strings.

I had an Oracle support ticket for a similar issue where they told me that the sequence of JDBC calls which BIRT uses isn't supported (in particular, the steps where getParameterMetaData is called). It even caused totally different SQL statements (made up from the original statement) to be parsed at the DB server side, which I found by tracing the DB session. Luckily, the different statements are only parsed, not executed, otherwise that could be a severe security issue. IMHO this is an issue inside the Oracle JDBC driver, which AFAIK this is still not fixed. This PR might circumvent that issue as well.

Next I tested the same report with Thomas' modifications of /org.eclipse.birt.report.data.oda.jdbc/src/org/eclipse/birt/report/data/oda/jdbc/ParameterMetaData.java.

I could see in the debugger that the new isFeatureNotSupported method is called, but the result is the same as before: The report fails with Unsupported feature: checkValidIndex.

hvbtup avatar Feb 03 '22 09:02 hvbtup

This is the report I used for testing - you'll have to adapt the JDBC properties of course oracle_null_parameters.zip

@tkrautinger Can you please test again? Your PR does not work for me.

hvbtup avatar Feb 03 '22 09:02 hvbtup

@hvbtup include for 4.10 or move to 4.11? I want to make a release.

wimjongman avatar Sep 19 '22 14:09 wimjongman

When I last tested it, it didn't work for me (see above), and didn't receive some explanation from @tkrautinger. So I'm -1 on including it into 4.10.

hvbtup avatar Sep 19 '22 14:09 hvbtup

So I'm -1 on including it into 4.10.

-1 from me too, so moving to 4.11

wimjongman avatar Sep 19 '22 14:09 wimjongman

The original issue for this change is done with Issue #1365 & PR #1371. It can be reopend if further changes of the message handling is necessary.

speckyspooky avatar Aug 06 '23 12:08 speckyspooky