Adarsh-Me commented on PR #13566:
URL: https://github.com/apache/ignite/pull/13566#issuecomment-5799900881

   @zstan Thanks for actually running it — that is the first execution data 
this PR has ever had, and it says the change is not sufficient as written. To 
be blunt about what I can and cannot stand behind here: the description says I 
never built or tested this, and I still cannot. Only GitHub's compile jobs have 
run (`Check java code on JDK 17` passes, so it builds); nothing that starts a 
node has touched it. So I have no green result to argue with yours, and I am 
not going to push a second speculative commit against a symptom I cannot 
reproduce.
   
   On the layout question, what I can answer factually: there is no `./sqlline` 
directory in an Ignite source checkout, so `ls -al ./sqlline` is empty here. 
The pieces are:
   
   - `modules/sqlline` — artifact `ignite-sqlline`, and it pins `sqlline` to 
**1.12.0** (`modules/sqlline/pom.xml:41-42`). So 1.12.0 is not a tool you opted 
into, it is what the distribution ships; that also means 1.9.0-vs-1.12.0 is a 
real behaviour difference inside the supported path, not a misconfigured client.
   - The class this PR changes is in 
`modules/core/src/main/java/org/apache/ignite/internal/jdbc/thin/JdbcThinResultSet.java`,
 i.e. it ships inside **ignite-core**. So for a patch to be visible to sqlline, 
the `ignite-core.jar` on that classpath has to be the one you just rebuilt — an 
`ignite-core` from a released zip sitting earlier on the classpath would 
silently win and reproduce exactly your "no change". That is the one thing I 
cannot see from here, and it is the only part of what you asked for that would 
actually discriminate.
   
   What I did check in source, rather than guess: `getTables` can return a 
zero-row result set for reasons this patch cannot touch — 
`JdbcThinDatabaseMetadata.java:745` returns `new JdbcThinResultSet(emptyList, 
meta)` before sending any request when the catalog is not accepted or 
`tblTypes` names neither TABLE nor VIEW. A headers-only table is exactly what 
that looks like. But the catalog branch does not fire from a plain connection: 
`JdbcThinConnection.getCatalog()` returns `null` (`:725-729`) and 
`isValidCatalog` accepts null (`:1461-1463`). So that path needs SQLLine 1.12.0 
to supply a non-null catalog of its own, which I cannot confirm from this host 
— there is no `sqlline` jar here for me to inspect, and I would rather say that 
than pretend I checked.
   
   Two things that would settle it, whichever is cheaper for you:
   
   1. The classpath/timestamps behind your run, so we know which `ignite-core` 
answered.
   2. Does `!tables` return rows against your patched driver under SQLLine 
1.9.0? That splits "driver still wrong" from "1.12.0 reads metadata a different 
way", which is where my analysis is currently stuck.
   
   And if you can schedule a real build of this head on `ci.ignite.apache.org`, 
`JdbcThinResultSetSelfTest` is the test that needs a live node — I cannot start 
one here. If it turns out 1.12.0 is asking something the driver genuinely does 
not answer, the right fix is likely elsewhere in `JdbcThinDatabaseMetadata` and 
I would rather close this PR than widen it on a guess.


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to