Query execute argument horror

every time i’m using queryExecute after a while, i always make code like this

queryExecute(sql:"select * from test",datasource:"myds");

instead of

queryExecute(sql:"select * from test",options:{datasource:"myds"});

what you think to support this directly and not only via “options” what is horrible, maybe as hidden arguments.

1 Like

so it will work either way? I mean, also as options? if so, then it’s fine by me.

sure, necessary for backward compatibility

Is this going to end up with just something like the script implementation where every option is also available at the top level?

Also what would happen in the following case

QueryExecute( sql: "select * from test" , datasource: 'dsn_1' , options: { datasource: 'dsn_2' } );

I mean based off CSS (ok maybe bad example) I’d be expecting the datasource to be dsn_1, but based off being able to supply a structure of options I’d hope for the datasource to be dsn_2. Admittedly the most likely cause for this is inconsistent coding, but who hasn’t come across inconsistency in their older coding from time to time!

This is my concern.

I think it’s fine as-is; I don’t think the function should be catering to shortfalls in Micha’s memory, TBH. I don’t mean that to be blunt, I just don’t think it’s a well-justified use case to clutter-up the function.

I think it’s telling that Micha goes as far to say “maybe as hidden arguments”. That is a really awful suggestion.

However I recommend raising a ticket and processing it as per usual so as to enable it getting in front of the TAG.

The issue is about improving a questionably sub-par original implementation and my gut says that I’m all for that. Yes, should go through TAG. Ability to improve syntax of functions while remaining backward compatible I think is a good thing; I’d personally be OK with the downside of having a ‘deprecated’ ‘options’ argument that will stick around.

Swapping out an options collection for a swarm of individual arguments is not an improvement. The current implementation is sensible and coherent. The thing has three args:

  • SQL
  • params for SQL
  • options for the connection

That’s a sensible breakdown, and also a sensible aggregation of the arguments. And I mean that in both directions: it’s neither too many nor too few args.

Still: we can just disagree on that. I trust the (overall) TAG’s judgement.

I simply assume that if something happen to me more than once, it happens to others as well. Having a second level for options like this is very uncommon in CFML, specially for functions.

1 Like

I find it hard to justify the options argument as a sensible breakdown. The abstraction of individual arguments into a single argument has questionable benefit and makes documentation fuzzier (there is no current structural way for documenting sub-struct keys other than a free text documentation). The documentation currently will say that the function takes three arguments, but that is misleading. Wrapping up arguments into a single collection argument does not reduce the complexity of a function; it only hides the true signature.

QueryExecute(
      sql        = "select id from somewhere"
    , datasource = "somedsn"
);

This is clear and in keeping with the rest of CFML. If we had started this way and were proposing changing it to bundle up arguments into a sub-set of arguments, I imagine the opposition to that change would be much stronger.

Because the functions usually don’t have… over a dozen possible individual values to pass.

If you look at <cfquery> it’s clear there’s three sets of “things” to pass:

  • the SQL statement within the tags
  • the params from within the tags
  • the attribute/value pairs from the opening tag

What you’re suggesting is akin to having this sort of thing:

<cfquery>
    <cfdatasource>
    <cfusername>
    <cfpassword>
    <!--- etc --->
</cfquery>

Some other ppl possibly do have the same glitch that you have. I make mistakes with the composition of the struct for the params all the time, but I just figure I need to buck my ideas up, not suggest the language changes to accommodate my personal failings.

Perhaps what you need is an IDE macro to write it out for you? That’s how I solve some of my repeated “why won’t I just bloody learn?” failings.

Again, changing the signature of a function because of a brain fart is not grounds for changing the functionality. “Having a second level for options like this is very uncommon in CFML, specially for functions.” is the sort of thing that might be grounds for a change of functionality, but I personally don’t think that’s a very well-formed case. As always: YMMV.

Good point. I was more meaning in contrast with Micha’s suggestion, and the division of the three there are, not necessarily within that third single arg. But that wasn’t at all clear.

When I first started to float an implementation of this with Adobe (I was not the only one doing so), I could see a case for this division within the “config” stuff:

  • datasource: datasource, user, password, dbtype
  • cache: cachedAfter, CacheId, cacheRegion, cachedWithin
  • connection: blockfactor, fetchClientInfo, maxRows, timeout, result (not sure about the latter)
  • ormOptions

That leaves debug hanging by itself. Does it make sense to stick it in datasource or connection? [shrug]

So that gives this function signature:

Query queryExecute(String sql[, Struct/Array parameters[, Struct datasource[, Struct cache[, Struct connection[, Struct ormOptions]]]]])

Six args is quite a lot. But I guess only the first one is required (even if the second would always been strongly recommended).

But there is still a case for one argument which is all the config options. Like it or not, they do have the cohesion that they are all config for how the query execution is performed.

Well that’s a bit of a logical fallacy at work there, innit? The popular position is not necessarily the most informed one. And especially in stuff like this, I find the popular position is often pretty wrong-headed.

I’m also not sure yer actually right about that. Still: no point dwelling on made-up alternate realities is there?

Also bear in mind that the current implementation did not pop into existence of its own accord: an active decision was made to implement it the way it was. And - thusfar - Micha’s been the only one I’ve seen questioning it, and for pretty dubious reasons. On the whole everyone’s been pretty pleased with the implementation as far as I can tell, except for the reasonable case that it’s still not as tidy as <cfquery>.

Taking this a different direction, I really wish there were a more fluent
api for queries. Something like:

result =
Query::from(datasource).select(columns).where(clause).execute(queryParams);

@dajester2013 that is an other topic worth discussing, like you know we have a couple of component under org.lucee.cfml Feed,FTP,Query …, we should for sure extend, this components for more practical use, no question. Could you raise an other thread for this?

@adam_cameron i have raised this Ticket to find out if others also think that this function could use improvement. When I’m in the end the only one having trouble with this function fine. But the only way to find out is to raise a thread and talk about.

@micstriit done. and btw, the one time i reply in email and think its going to google groups, it comes here instead… otherwise i would have started with it as a linked topic to begin with.

as far as this thread though, the queryExecute signature is

queryExecute(String sql, Array|Struct params, Struct options);

Correct? I think Micha’s suggestion works if you consider an “alternate” method signature:

queryExecute(String sql, String datasource, Array|Struct params);

Now, as far as CFML goes, it might be “cluttered” as far as arguments go, but this is not an uncommon thing for say, Java. As long as you could pass the arguments without names:

queryExecute("select * from table", "datasource");

Absolutely!

Different thread, eh?

6 posts were split to a new topic: Query execute with IN