On Mon, Jan 14, 2019 at 11:34 AM Tom Lane <t...@sss.pgh.pa.us> wrote: > > James Coleman <jtc...@gmail.com> writes: > > On Mon, Jan 14, 2019 at 11:08 AM Tom Lane <t...@sss.pgh.pa.us> wrote: > >> I think these test cases don't actually prove much about the behavior > >> of your patch. Wouldn't they be handled by the generic OR-conversion > >> logic, since there's fewer than MAX_SAOP_ARRAY_SIZE items? > > > Which ones are you looking at in particular? The "inline" (non-cte) > > happy and sad path cases have 102 total array elements (as does the > > happy path cte case), and MAX_SAOP_ARRAY_SIZE is 100. The other two > > tests are about the empty array case so much have 0 items (and were > > the ones that showed different behavior between v3 and v4). > > I was looking at the empty-array ones. I don't see how that reaches > your patch; we should not be doing predicate_implied_by_simple_clause > on the ScalarArrayOp itself, because predicate_classify should've > decided to treat it as an OR clause.
At one point I was getting all 'f' instead of all 't' for those, but now I can't reproduce it going from either master to my patch or to my patch without the element count checks, so I'm not sure how/when I was getting that anymore. Are those invariants we want to keep (and recognize as regression tests)? If so, I can confirm that they aren't duplicative of the rest of the file, and if not I can remove them. James Coleman
