Close before Open

[aehimself]

it is the users responsibility to know that he modified properties.

Exactly my thoughts. This is why I said at the first place - if someone is re-using a query object for different resultsets - just call .Close before changing the command. Worst case scenario it won't do anything.

When it comes to modifying the current SQL statement, I tink that we should forbid changing it if there is a technical reason to do so.

This is where we disagree. It's not about the technical limitations, it's about making sense. This is why this topic is causing me headaches - I simply can not tell a simple scenario when changing the .SQL makes sense when we have a dataset open.
If we allow this it'll only open up endless trapdoors... If we have a select statement and a dataset open, change the SQL to UPDATE, INSERT or DELETE and call .ExecSQL what is supposed to happen? Keep the dataset and execute the new statement? Raise an exception as the dataset is open? If the later - why do we allow to change it in the first place?

If you are responsible for designing / assembling a car, there are no technical difficulties to install an eject mechanism on the driver's seat when you accelerate hard. Or a rocket-powered turbo. We can put blades all around our cars just like they did in Carmageddon, but we are not.
You see where I'm going with this I suppose.

So - this means to decide wether we should forbid changing the SQL statement or not, we should first know if there is a technical reason to forbid it.

See above. It does not make sense - at least to me.

Another option migt be this one: If somebody changes the SQL statement we could set a flag FSqlHasChanged or something like this. If somebody calls Refresh and FSqlHasChanged is true, we could raise an exception, that says "Sorry, guy - you changed the SQL statement, so we cannot do a refresh."

The KISS coding approach says otherwise. Why to have one more flag, one more thing to consume memory, one more thing to consider if we simply can stop the avalanche before it even starts?


Final thoughts:
Being only a contributor the choice is not mine; as I previously stated: if you think the modification does not fit the codebase roll it back. In my opinion this topic was created because of a "user" error and could have been avoided if the code is well written - it won't affect any of my programs in any way.
But looking at the test suite .Close was occasionally called before changing the SQL while the dataset is open; see ZTestCompADOBugReport.TestTrailingSpaces for example.

Just please let me know if I should continue fixing the tests as I have no intention of spending hours on something which will be discarded at the end.

[marsupilami]

Being only a contributor

Here we disagree. You aren't. Maybe we should have said that more early. Have a look at your rank - it should denote something new now

But honestly - we would have granted you commit rights long ago. Also you always have valuable input for discussions. So I think this is overdue. Do you have a username on sourceforge? Then I could grant you commit rights there.

the choice is not mine;

Now it is yours

as I previously stated: if you think the modification does not fit the codebase roll it back. In my opinion this topic was created because of a "user" error and could have been avoided if the code is well written - it won't affect any of my programs in any way.
But looking at the test suite .Close was occasionally called before changing the SQL while the dataset is open; see ZTestCompADOBugReport.TestTrailingSpaces for example.

Just please let me know if I should continue fixing the tests as I have no intention of spending hours on something which will be discarded at the end.

Since I am undecided, we take your position and see what our users say. But we will have to do it for 8.0 only to not break backwards compatibility for 7.2 and we will have to document it in the release notes.
So - please go on.

[aehimself]

Here we disagree. You aren't. Maybe we should have said that more early. Have a look at your rank - it should denote something new now ;)

Oh, wow... does a salary raise come with the promotion too? :D

I remember the first application I built with Zeos 6.x many-many years ago... I never thought I will actually get a status / rank like this with you guys. I feel like I'm not contributing much (neither discussions, neither code) and I am still far away from knowing the majority of the internal logic of Zeos but with time comes knowledge.

With all seriousness thank you very much, it means a lot.

Do you have a username on sourceforge? Then I could grant you commit rights there. ;)

I don't have a SF account. If it's not a big issue for you I'd like to stick to the GitHub pull request way. At least I know how that works and I'm not going to mess things up that badly :)

Since I am undecided, we take your position and see what our users say. But we will have to do it for 8.0 only to not break backwards compatibility for 7.2 and we will have to document it in the release notes.
So - please go on. :)

I should have thought of this a long time ago. I can put this dilemma (maybe even with a poll) on DelphiPraxis - there are a lot of very smart people there and a huge userbase for the result to be viable - even if the participants do not use Zeos.
In the mean time I'll continue with the tests. I ask for your patience, the work around the house is consuming most of my time and energy since I arrived back home :S

Edit: Poll created and can be accessed here.

[Fr0sT]

Oh, wow... does a salary raise come with the promotion too?

Sure, you get +50% of your previous amount

Well, my opinion is that any class is responsible for keeping its internal state consistency. Good example is TStringList: Sorted property not only changes a field, it sorts all items and keeps newly added items in order. Honestly I can't imagine a case when someone could need to have new SQL with old resultset. So I vote for disallowing this state in any way (rejecting SQL, auto disposing result set, closing, etc). What way exactly - that is the question, here we have to consider backward compat as well. I'd reject SQL change on an open dataset as I feel an allergy to such kind of auto behavior as implicit closing or even re-opening. Just my 5 knuts

[marsupilami]

Honestly I can't imagine a case when someone could need to have new SQL with old resultset.

One use case: You want to write an interactive SQL application. Something like FlameRobin or MS SQL Server Studio or ... In that case you want to show the last result set while already working on the new SQL. This is easily possible as long as you don't use parameters because then one only needs to assign the SQL when the user presses the "Run SQL" button.
But if you want to use parameters, and want to simply ask TZQuery what the parameter names in the current SQL statement are, using TZQuery.Params, things get complicated.
Currently one can assign the SQL and get the new parameter names. If we raise an exception, one has to use two TZQueries and manage which one is active and which one gets used for the next SQL statement. Sure - this is quite easy. But it complicates the use case

[marsupilami]

Here we disagree. You aren't. Maybe we should have said that more early. Have a look at your rank - it should denote something new now

Oh, wow... does a salary raise come with the promotion too?

Sure - I agree to F0st - you get a 100% increase in salary

Do you have a username on sourceforge? Then I could grant you commit rights there.

I don't have a SF account. If it's not a big issue for you I'd like to stick to the GitHub pull request way. At least I know how that works and I'm not going to mess things up that badly

Sure - we can keep it like that

I can put this dilemma (maybe even with a poll) on DelphiPraxis - there are a lot of very smart people there and a huge userbase for the result to be viable - even if the participants do not use Zeos.

I like that idea

I ask for your patience, the work around the house is consuming most of my time and energy since I arrived back home :S

Sure - no problem. The release notes don't get as much progress as I want them to anyway

[aehimself]

One use case: You want to write an interactive SQL application. Something like FlameRobin or MS SQL Server Studio or ... In that case you want to show the last result set while already working on the new SQL. This is easily possible as long as you don't use parameters because then one only needs to assign the SQL when the user presses the "Run SQL" button.

Well, my most active application nowdays happens to be exactly this :) The idea was borrowed from the mainstream (SQL Management Studio, PL/SQL, MySQL Workbench) - one tab does one thing but you can have multiple tabs open to the same database:

multisql.PNG

When an SQL is executed the main ZQuery closes, updates the .SQL property and checks for parameters to show the input window.

Not even my scenario has the need to change the .SQL while a dataset is open.

[Fr0sT]

One use case

Well, how many interactive SQL apps are being written today in Delphi+Zeos? This use case is tooooo rare for me. And it's easily solved by storing parameters in separate list.

[marsupilami]

One use case

Well, how many interactive SQL apps are being written today in Delphi+Zeos? This use case is tooooo rare for me. And it's easily solved by storing parameters in separate list.

I do know about three such applications: One written by aehimself (see above), one written by me and one written by Fantablup.

I don't say, we have to optimize for that use case. I can happily live without this and I know this can be solved by having two queries - one for determining parameters as the user writes the SQL and one for executing the SQL. I just wanted to say that there is a use case

[marsupilami]

@aehimself: Wow - that really looks neat.

When an SQL is executed the main ZQuery closes, updates the .SQL property and checks for parameters to show the input window.

Yes - your scenario is a three step scenario. My idea was different. I had the idea to have a panel that shows the parameters. I want to show parameters there as soon as the user writes them in his SQL. This requires to constantly push the current SQL statement into a TZQuery object and see what parameters it returns. I don't want to show a separate window for asking for parameters.

[aehimself]

Yes - your scenario is a three step scenario. My idea was different. I had the idea to have a panel that shows the parameters. I want to show parameters there as soon as the user writes them in his SQL. This requires to constantly push the current SQL statement into a TZQuery object and see what parameters it returns. I don't want to show a separate window for asking for parameters. ;)

Ah, that makes sense. I'm using a modal window to query the parameters based on their type (all can be nullable if you start with :p0):

multisql2.PNG

Do you need a TQuery just to parse parameters? Can't it be done with a single TZSQLStrings instance? I mean it is the one triggering rebuilding the parameter list in Zeos, and it has the necessary public properties:

Code: Select all

    property ParamCount: Integer read GetParamCount;
    property ParamChar: Char read FParamChar write SetParamChar;
    property ParamNames[Index: Integer]: string read GetParamName;

[marsupilami]

I didn't know that. Using a TZQuery and inspecting TZQuery.Params seemed liek the most simple way to get that done

[aehimself]

@ Jan:
The original pull request now contains a second commit as an attempt to fix some exceptions caused by the .SQL change exceptions.

Please merge them so I can see if I missed any... as I don't have most of the RDBMS flavors at home I can not run a complete ZTestAll.
It compiles though so at least I did not break anything :)

[aehimself]

@ Jan:

I saw that Jenkins got unstuck and was ready with a set of new test results. I sent a pull request to fix the remaining tests which failed with "Resultset is already open".

[marsupilami]

I applied the patch At a first glance it seems that everything works as expected