Re: Explicit psqlrc

On Jul 19, 2010, at 10:40 PM, Robert Haas wrote:

> On Wed, Jun 23, 2010 at 9:22 AM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>> On Wed, Jun 23, 2010 at 9:17 AM, gabrielle <gorthx(at)gmail(dot)com> wrote:
>>> On Mon, Jun 21, 2010 at 6:16 PM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>>>> Well, that might be a good idea, too, but my expectation is that:
>>>>
>>>> psql -f one -f two -f three
>>>>
>>>> ought to behave in a manner fairly similar to:
>>>>
>>>> cat one two three > all
>>>> psql -f all
>>>>
>>>> and it sounds like with this patch that's far from being the case.
>>>
>>> Correct. Each is handled individually.
>>>
>>> Should I continue to check on ON_ERROR_ROLLBACK results, or bounce
>>> this back to the author?
>>
>> It doesn't hurt to continue to review and find other problems so that
>> the author can try to fix them all at once, but it certainly seems
>> clear that it's not ready to commit at this point, so it definitely
>> needs to go back to the author for a rework.
>
> Since it has been over a month since this review was posted and no new
> version of the patch has appeared, I think we should mark this patch
> as Returned with Feedback.

Sorry for the delays in response. This is fine; I think there are some semantic questions that should still be resolved at this point, particularly if we're moving toward supporting multiple -c and -f lines as expressed (an idea that I agree would be useful). Specifically:

1) Does the -1 flag with multiple files indicate a single transaction for all commands/files or that each command/file is run in its own transaction? I'd implemented this with the intent that all files were run in a single transaction, but it's at least a bit ambiguous, and should probably be documented at the very least.

2) I had a question (expressed in the comments) about how the final error handling status should be reported/handled. I can see this affecting a number of things, particularly ON_ERROR_{STOP,ROLLBACK} behavior; specifically, if there was an error, it sounds like it should just abort processing of any other queued files/commands at this point (in the case of ON_ERROR_STOP, at least).

3) With the switch to multiple intermixed -c and -f flags, the internal representation will essentially have to change to a queue of structs; I think in that case, we'd want to modify the current .psqlrc handling to push a struct representing the .psqlrc file at the front of the queue, depending on the code paths that currently set this up. Are there any gotchas to this approach? (I'm looking essentially for odd code paths where say .psqlrc was not loaded before, but now would be given the proper input of -c, -f file, -f -.)

I'll look more in-depth at the posted feedback and Mark's proposed patch.

Regards,

David
--
David Christensen
End Point Corporation
david(at)endpoint(dot)com