Multiple setup steps for isolation tests

Started by Kevin Grittnerover 13 years ago6 messageshackers
Jump to latest
#1Kevin Grittner
Kevin.Grittner@wicourts.gov

I just today found that the index-only scan feature has broken SSI.
I don't think it will take much to fix, and I'm looking at that, but
the first thing I wanted was a test to show the breakage. I couldn't
find a way to do that without running VACUUM after loading data to
the test tables, and because VACUUM refuses to run in a
multi-statement batch I propose the following patch to the isolation
testing code, which allows multiple setup blocks. Using this code I
now have an isolation test to show the breakage.

If there are no objections, I will apply this to HEAD and 9.2.

I'm working on a fix to the bug itself.

-Kevin

Attachments:

isolation-setuplist-v1.patchapplication/octet-stream; name=isolation-setuplist-v1.patchDownload+36-18
#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Kevin Grittner (#1)
Re: Multiple setup steps for isolation tests

"Kevin Grittner" <Kevin.Grittner@wicourts.gov> writes:

I just today found that the index-only scan feature has broken SSI.
I don't think it will take much to fix, and I'm looking at that, but
the first thing I wanted was a test to show the breakage.

Ugh. That sounds like a release-blocker. What's your ETA for a fix?

I couldn't
find a way to do that without running VACUUM after loading data to
the test tables, and because VACUUM refuses to run in a
multi-statement batch I propose the following patch to the isolation
testing code, which allows multiple setup blocks. Using this code I
now have an isolation test to show the breakage.

If there are no objections, I will apply this to HEAD and 9.2.

The grammar changes look wrong: I think you eliminated the ability to
have zero setup steps, no? Instead, setup_list should expand to either
empty or "setup_list setup".

regards, tom lane

#3Kevin Grittner
Kevin.Grittner@wicourts.gov
In reply to: Tom Lane (#2)
Re: Multiple setup steps for isolation tests

Tom Lane wrote:
"Kevin Grittner" writes:

I just today found that the index-only scan feature has broken
SSI. I don't think it will take much to fix, and I'm looking at
that, but the first thing I wanted was a test to show the
breakage.

Ugh. That sounds like a release-blocker. What's your ETA for a fix?

I have a fix now. I just got done testing it. I will post right
after this, and can apply as soon as I know there are no objections.

I couldn't find a way to do that without running VACUUM after
loading data to the test tables, and because VACUUM refuses to run
in a multi-statement batch I propose the following patch to the
isolation testing code, which allows multiple setup blocks. Using
this code I now have an isolation test to show the breakage.

If there are no objections, I will apply this to HEAD and 9.2.

The grammar changes look wrong: I think you eliminated the ability
to have zero setup steps, no? Instead, setup_list should expand to
either empty or "setup_list setup".

I tried that first, but had shift/reduce conflicts. I noticed that
there were no *tests* without setup so far, and it's hard to imagine
when that would be sensible, so I didn't feel too bad requiring the
setup list for the test but leaving a single, optional, setup for
each connection. If you can suggest how I could move to a list and
still keep it optional without the shift/reduce problems, I'd be
happy to do it. I just didn't see any obvious way to do it. But
then, I haven't done a lot in flex.

New version of this patch attached. I think the only change is that
I modified the README file.

-Kevin

Attachments:

isolation-setuplist-v2.patchapplication/octet-stream; name=isolation-setuplist-v2.patchDownload+44-26
#4Tom Lane
tgl@sss.pgh.pa.us
In reply to: Kevin Grittner (#3)
Re: Multiple setup steps for isolation tests

"Kevin Grittner" <Kevin.Grittner@wicourts.gov> writes:

Tom Lane wrote:
The grammar changes look wrong: I think you eliminated the ability
to have zero setup steps, no? Instead, setup_list should expand to
either empty or "setup_list setup".

I tried that first, but had shift/reduce conflicts.

[ scratches head ... ] Dunno what you did exactly, but the attached
version works fine for me.

regards, tom lane

#5Kevin Grittner
Kevin.Grittner@wicourts.gov
In reply to: Tom Lane (#4)
Re: Multiple setup steps for isolation tests

Tom Lane wrote:
"Kevin Grittner" writes:

Tom Lane wrote:
The grammar changes look wrong: I think you eliminated the
ability to have zero setup steps, no? Instead, setup_list should
expand to either empty or "setup_list setup".

I tried that first, but had shift/reduce conflicts.

[ scratches head ... ] Dunno what you did exactly, but the attached
version works fine for me.

[ slaps forhead ]

Yeah, that should do it. Will apply.

Thanks.

-Kevin

#6Kevin Grittner
Kevin.Grittner@wicourts.gov
In reply to: Kevin Grittner (#5)
Re: Multiple setup steps for isolation tests

"Kevin Grittner" wrote:

Tom Lane wrote:

the attached version works fine for me.

Yeah, that should do it. Will apply.

Pushed to master and REL9_2_STABLE.

-Kevin