vrogier / ocilib

OCILIB (C and C++ Drivers for Oracle) - Open source C and C++ library for accessing Oracle databases
http://www.ocilib.net
Apache License 2.0
325 stars 119 forks source link

Optional support of Oracle substitution variables #43

Closed dbpm closed 7 years ago

dbpm commented 8 years ago

In SQL*Plus code (as well as for some IDEs, and libraries, like DevArt ODAC) it's possible to specify "substitition" variables, like shown below

SELECT * FROM DUAL WHERE &subval

where subval can be presented as remaining part of SQL text, lile text " NOT EXISTS (SELECT NULL FROM ... " and so on. Most close analogue are C macroses.

Those variables are well-described here: https://blogs.oracle.com/opal/entry/sqlplus_101_substitution_varia

we need to implement such optional support for ocilib as well (it MUST be disabled by default for compatibility reason).

Oracle's OCI level seems does not support those substitition variables, from my understanding such support could be implemented at client level only

vrogier commented 8 years ago

Hi,

Can't you use OCI_ExecuteFmt() or OCI_PrepareFmt() methods that accepts the token %m ? It performs the same feature.

Vincent

vrogier commented 8 years ago

Example:

OCI_ExecuteStmtFmt(st, "select sysdate from %m", "dual");

I think it addresses your request

Regards,

Vincent

dbpm commented 8 years ago

Technically that is related, but is not the same. Point is to support SQL*Plus scripts as is. %m is too far away

FladoToo commented 8 years ago

Sorry to intrude, but I don't think OCILIB should try to implement SQL*Plus. There's much more to supporting SQL*Plus scripts than substitution variables. And I, for one, do not need all that code in my programs.

dbpm commented 8 years ago

If you don't understand how to use macroses (the substitution variables) - it does not mean that this feature is not required at all. For more mature libraries this feature is standard and has been available for years:

You can see it there, for an example: https://www.devart.com/odac/docs/?work_macros.htm

vrogier commented 8 years ago

Basically these macros provided by devart, for example, are equivalent to %m token when using OCI_PrepareFmt().

Can you point what you can do with devart macros and you can't do in OCILIB ?

dbpm commented 8 years ago

It's not equalent.

%m implies hardcoded order of passwed variables into the function call.

while the macroses/substitution variables are dynamic - that way I can write code which will handle any SQL, any number of variables, depending on user imput.

also %m is positional parameter (order matters), and for multiple instances of the same variable, like

CREATE TABLE test TABLESPACE &tablespace  lob (c) STORE AS BASICFILE (TABLESPACE &tablespace)

vs

CREATE TABLE test TABLESPACE %m lob (c) STORE AS BASICFILE segname (TABLESPACE %m)

If will have to pass tablespace name twice.

For some cases the task will become even unreachable, because number of partitions can vary, and I can't predict how many %m parames I will have to pass into function (for macroses approach I will have to pass just single variable &tablespace, which will be instantiated multiple times in resulting code)

CREATE TABLE sales
  ( prod_id       NUMBER(6)
  , cust_id       NUMBER
  , time_id       DATE
  , channel_id    CHAR(1)
  , promo_id      NUMBER(6)
  , quantity_sold NUMBER(3)
  , amount_sold   NUMBER(10,2)
  )
 PARTITION BY RANGE (time_id)
 ( PARTITION p2015 VALUES LESS THAN (DATE '01-APR-2006') TABLESPACE &tablespace._p2015
 , PARTITION p2016 VALUES LESS THAN (DATE '01-JUL-2006') TABLESPACE &tablespace._p2016
 , PARTITION p2017 VALUES LESS THAN (DATE '01-OCT-2006') TABLESPACE &tablespace._p2017
 , PARTITION p2018 VALUES LESS THAN (DATE '01-JAN-2007') TABLESPACE &tablespace._p2018
 );

So %m similar, but not even close to the macroses power, and does not meet our needs.

Will you object against pull request? I can prepare code next week (I had been implemented, but I need to review/merge from ocilib 3.12x branch, it could take some time).

FladoToo commented 8 years ago

Please, don't turn OCILIB into a generic string processing library!

vrogier commented 8 years ago

By default ocilib does not parse input sql strings, unlike most other libs (such as otl for instance) for performance reasons. Of course i could add something like oci_setsqlvariable() and statement objects could check if any are stored and then perform substitutions. Why not. But it will still impact all statements even with ni macro in sql statement...

vrogier commented 8 years ago

I will see what i can do about it.

I would interested in your definition of more mature libraries ? Aging ? Feature coverage ? Power ? Bug free ? More expensive ? Commercial ? What else ?

First, ocilib is the only open source oracle library running on all oracle platforms and accessible from any language. Second, it supports 99 % of oci api features. Rare are the libraries to provide a such coverage, especially open source ones. Third, ocilib is about 9 years old and used in pretty much in every computing areas where oracle is involded, from medical, banking, industries, transports, and so on. Thus I consider ocilib as a mature library. Fourth, when you compare devart odac features with ocilib, the gap is pretty smalll in the scope of oci support. Last, but not least, , devart odac cost minimum 1500 $ to start working with. And they must have an deal agreement with oracle corp in order to provide an implementation of oracle wired protocol (otherwise it's illegal). Ocilib costs zero. They run a business, not me.

Thus, you cannot compare an open source library with a commercial product. Especially when this product is only available in C++ and Pascal where as the open source run anywhere from any language. Personally, i think that ocilib does not have much to envy to odac.

I'm opened as usual for any bug report or enhancement request, for sure : )

I will study your request about variables (and other ones) as I did for your request about handling strings with null characters that I've addressed.

It is just that, filling suddently the tracker with enhancement requests stating that the lib seems not mature enought and shall support features brought by commercial products for free, is a bit abrupt ; )

Adding variable sibstitution support is quite simple and requires only few methods. For exposing OCINumber, it requires a more consequent work, especially for the C++ api. For direct path, I need to investigate what I can do.

I will update each request asap with more information !

Best regards

vrogier commented 8 years ago

Could the following suite your needs ?

New methods

OCI_VariableStore * OCI_VariableStoreCreate();
boolean OCI_VariableStoreFree(OCI_VariableStore *store);
boolean OCI_VariableStoreClear(OCI_VariableStore *store);
boolean OCI_VariableStoreSetValue(const otext *name, const otext *value);
const otext * OCI_VariableStoreGetValue(const otext *name);
boolean OCI_VariableStoreAddStatement(OCI_VariableStore *store, OCI_Statement *stmt);
boolean OCI_VariableStoreRemoveStatement(OCI_VariableStore *store, OCI_Statement *stmt);

Use case

OCI_VariableStore *store = OCI_VariableStoreCreate();
OCI_VariableStoreSetValue("table", "all_users");
OCI_VariableStoreSetValue("column", "username");
OCI_Statement *st = OCI_StatementCreate(con);
OCI_VariableStoreAddStatement(store, st);
OCI_Prepare(st, "select &column from &table where &column = :1");
OCI_BindString(st, ":1", str, sizeof(str));
OCI_Execute(st);
OCI_VariableStoreRemoveStatement(store, st);
OCI_VariableStoreFree(store);
dbpm commented 8 years ago

Sorry for offtopic, I did not say that ocilib is not mature. I just said that ODAC libs (and users) have had the macroses for years, and it's a bit strange that ocilib still does not provide it (a bit less mature, but it is fixable, right? :).

Also those functions are available in FireDAC (AnyDAC derivative). http://docs.embarcadero.com/products/rad_studio/firedac/uADCompClient_TADQuery_Macros.html

So such macro idea is not something unique - as I know lot of people use its benefit.

All those Delphi libs are commerical ones, right, I don't know any production ready free one. Though I don't think that such things are related to the techical part of the issue.

dbpm commented 8 years ago

I'd implement API as just single simple call:

OCI_BindSubstitution(st, "&tablename", str, sizeof(str)); 

and/or

OCI_BindMacro(st, "&tablename", str); // as far as the str is practically always NULL terminated string

OCI_Prepare(), subsequently, could see that at least one OCI_BindSubstitition() call was performed, so the underlying query is &substitution aware and developer knows what he does.

Once that "substitution aware" flag is set by OCI_BindSubstitition(), then the OCI_Prepare() can start building of the internal SQL string which will be finally passed to server. OCI_StatementClose() could dispose that internal string from heap later.

Adding the checking into the OCI_Prepare() and OCI_StatementClose()

 if (stmt->substituted)

practically will not affect performance; the stmt->substituted field will be false by default, of course.

Also please note that the macroses can be nested, multiple times, like

OCI_BindSubstitution(st, "&where", "&table..&column != 0"); 
OCI_BindSubstitution(st, "&table", "all_users"); 
OCI_BindSubstitution(st, "&column", "user_id"); 
OCI_BindSubstitution(st, "&expr", "'&where' expr"); 
OCI_Prepare(st, "select &&table..&column &table._&column, &expr from &table where &where");

and resulting query should to be built as follows (strings to be replaced in loop untill nothing will be found):

select all_users.user_id all_users_user_id,  'all_users.user_id != 0' expr from all_users where all_users.user_id != 0

please note as macro separator can be used any non-alphanumeric symbol (underscores are alphanumeric too), and dot symbol has special meaning: it's macro name terminator/separator, in case if you need to put alphanum character straight after the macro. It's described in SQL*Plus doc and first link above, in chapter 9.6 Appending Alphanumeric Characters Immediately After a Substitution Variable

FladoToo commented 8 years ago

I'm sorry, but I still don't see why this string manipulation functionality should be implemented in a database interface library. A regexp_replace (possibly in a loop until no more matches are found - to cater for nested macros) should do nicely. And just to clear any doubts here: yes, I understand perfectly well what SQLPlus substitution variables are and how to use them in SQLPlus. I also use the functionality you describe quite often in C - I just use a library function to do the macro expansions before I call OCILIB; I also use that same library function for other things, like generating localised boilerplate texts to display to the user - should I now go and refactor my code to use OCILIB's macro expansion for the strings that happen to be SQL, or maybe start insisting that the message display library implement macro expansion functionality, too? Please, do not complicate the API more than necessary and resist the scope creep - even if it is easy to implement now, that is code that needs to be maintained and debugged and regression-tested with every future change!

dbpm commented 8 years ago

FladoToo,

1) Regexp replacement is not the same, as well as %m. I need to use the same SQL text in SQL*Plus, TOAD, PL/SQL Developer, wherever. If each tool will use own implementation of the string substitution/macro expansion it will quickly become support and debug nightmare: rather than just copy/pasting values I will have to do the regexp replacement manually in SQL IDE. That is not convenient in general.

2) I don't understand which complication do you mean. We just need to add one single additional and optional call. If you don't need or don't want to use it - just don't make the call, nothing will change to you. The same is true for regressions and so on. Nothing changes, as it's just an enhancement. But of course we need to add... additional regression tests just for the new functionality.

UPDATED: 3) Message displaying need to show resulting SQL text, expanded one. The same is true for OCI_GetSql just because OCI_GetSqlErrorPos will point to nowhere if the OCI_GetSql will return SQL which was never passed to Oracle. Also I think there is no reason to keep original non-expanded SQL with &variables, because each OCI_BindSubstitution() call need to be followed by OCI_Prepare() in any case. I think nobody/nothing will need the SQL with &variable after the OCI_Prepare() call. Also it will be better to keep/return expanded SQL text everywhere for the compatibility reasons.

FladoToo commented 8 years ago

dbpm, I think there's some misunderstanding here. Let me try to clear it: 1) OCILIB is not a tool in the sense that SQLPlus, Toad, etc. are, and it hopefully never will be. You want some tool functionality incorporated in the library to make your debugging easier; I may want other tool functionality implemented (e.g., COPY, or set autotrace trace statistics, or @) using the exact same argument - where should the line be drawn? If you want an API that can take an SQLPlus script and execute it the same way SQL*Plus does, you've come to the wrong place. 2) more code=(potentially) more bugs, harder maintenance, more effort for new features ("yes, this new feature would be nice, but how it will work with substitution variables?") even if nobody uses it. 3) I mean messages unrelated to SQL, as in msg_en="Customer &c. is not allowed to do this"; msg_de="Kunde &c. darf das nicht"; I already have a function that expands the macro named "c" in these strings. I can use the same function for expanding macros in SQL.

dbpm commented 8 years ago

1) COPY is SQL*Plus only unique feature, no one known OCI related library implements it, the same is true for SET AUTOTRACE, @ and so on, nobody requests to support that, so referenning to them is just kind of counterproductive verbal speculation, sorry. 2) ok, less code (ideally - empty .c) produces no bugs and easier in maintenance, it's really good point, can't disagree 3) It's good to you that you already have this function, but probably it's good time to share the functionality to others, isn’t it?

FladoToo commented 8 years ago

1) Substitution variables are a SQLPlus feature, too. So your argument boils down to "some other OCI libraries have it, so OCILIB must have it, too." I personally don't think every library should make the same mistakes, so for me this argument is not convincing. 3) Sadly, the function in question is provided by a proprietary closed-source library, so I don't have the actual code. But my point is, it is a *string processing library, and not a OCI library or a message processing library.

Cheers! Flado On 25 May 2016 17:02, "dbpm" notifications@github.com wrote:

1) COPY is SQL*Plus only unique feature, no one known OCI related library implements it, the same is true for SET AUTOTRACE, @ and so on, nobody requests to support that, so referenning to them is just kind of counterproductive verbal speculation, sorry. 2) ok, less code (ideally - empty .c) produces no bugs and easier in maintenance, it's really good point, can't disagree 3) It's good to you that you already have this function, but probably it's good time to share the functionality to others, isn’t it?

— You are receiving this because you commented. Reply to this email directly or view it on GitHub https://github.com/vrogier/ocilib/issues/43#issuecomment-221585478

dbpm commented 8 years ago

every library should make the same mistakes

It's really hard to understand why you have decided that substitution/macros functionality it is a mistake. Macroses in C/C++ also are the kind of mistakes? Just because Java or Swift suddenly decided to do not implement them? Ok. Let's rewrite ocilib without any macroses, then let's see how the code will look.

I still don't get your point, sorry.

FladoToo commented 8 years ago

My point is simple: macro expansion is a string processing functionality which should be implemented in a string processing library. OCILIB is not a string processing library. Ergo, macro expansion has no place in OCILIB.

vrogier commented 7 years ago

Hi,

Quite an old topic. I don't have any short/mid term plans for implementing this request that is not really in the scope of the OCILIB library. I agree it would be a 'nice to have" but I've got many other stuff to do in OCLIB prior to that. Thus i'm closing this enhancement request for the moment.