fix sql_query_explain.Oracle

[thomashulst]

Fixes #1353

• use dbExecute for EXPLAIN PLAN FOR
• remove extra ) in DBMS_XPLAN.DISPLAY()

[mgirlich]

sql_query_explain() should not execute a query. I think that sql_query_explain.Oracle() should error with a helpful error instead and you add db_explain.Oracle(). Also please add a method for .

[thomashulst]

sql_query_explain() should not execute a query. I think that sql_query_explain.Oracle() should error with a helpful error instead and you add db_explain.Oracle(). Also please add a method for .

Thanks for your pointers, figuring this out as I go. I don't want to impose, but hope to work through this in the hopes of being able to contribute more frequently.

I just submitted something which is hopefully closer to what is intended. Currently db_explain() still falls back to db_explain.DBIConnection(), but I didn't want to touch that before knowing what I submitted is in the right direction.

[hadley]

It seems like the basic problem is that explaining a query plan in Oracle requires two steps — you first use EXPLAIN PLAN to save the plan to the DBMS_XPLAN.DISPLAY table, and then you need to retrieve the rows from that table. An initial useful contribution would be to document this with links to the Oracle docs, so we don't re-confused when we come back to this code in the future.

You also seem to have split the single SQL statement into two pieces, only one of which is generated by sqll_query_explain, but you don't explain why. If it's because it doesn't work as a single statement, I think it would be slightly clearer for sql_query_explain() to return a character vector, and then you can execute those pieces individually in db_explain().

I think you also need to consider what happens if multiple queries are explained in one session. Won't those plans continue to pile up in the same database? I think it would be useful to consider how you might keep them separate.

[thomashulst]

It seems like the basic problem is that explaining a query plan in Oracle requires two steps — you first use EXPLAIN PLAN to save the plan to the DBMS_XPLAN.DISPLAY table, and then you need to retrieve the rows from that table. An initial useful contribution would be to document this with links to the Oracle docs, so we don't re-confused when we come back to this code in the future.

I'll link to the Oracle documentation of the current long term release (19c). The gist of explaining and displaying execution plans in Oracle can be found here.

Basically, EXPLAIN PLAN inserts a row with the execution plan for a particular statement into the PLAN_TABLE. PLAN_TABLE is a global temporary table automatically set up for each user and the default output table of EXPLAIN PLAN. Then, DBMS_XPLAN.DISPLAY is used to display the execution plan. It's a function that takes a few arguments, but by default it selects the most recent execution plan from PLAN_TABLE and returns it nicely formatted.

You also seem to have split the single SQL statement into two pieces, only one of which is generated by sqll_query_explain, but you don't explain why. If it's because it doesn't work as a single statement, I think it would be slightly clearer for sql_query_explain() to return a character vector, and then you can execute those pieces individually in db_explain().

Exactly, it doesn't work as a single statement. I see why returning a character vector might be nicer and will submit that shortly. Again, db_explain still falls back to db_explain.DBIConnection(), but the commit will hopefully be closer to what is expected.

I think you also need to consider what happens if multiple queries are explained in one session. Won't those plans continue to pile up in the same database? I think it would be useful to consider how you might keep them separate.

In theory you could specify a unique value for the STATEMENT_ID when using EXPLAIN PLAN and then retrieving it with DBMS_XPLAN.DISPLAY(statement_id => unique_id). However, the DBMS_XPLAN.DISPLAY function selects the most recently generated execution plan by default, so in my mind it takes care of this problem. Furthermore, PLAN_TABLE is dropped at the end of each session, so no need to worry about plans piling up.

Thanks again for your patience and all helpful pointers.

[hadley]

Thanks for the explanations!

[hadley]

@thomashulst thanks so much for this PR!

[thomashulst]

Happy to contribute, I learned a lot from it 😊

[hadley]

If you're interested in tackling another Oracle-dbplyr issue, I think #750 looks fairly straightforward.