Re: Teach predtest about IS [NOT] <boolean> proofs
Tom Lane <tgl@sss.pgh.pa.us>
From: Tom Lane <tgl@sss.pgh.pa.us>
To: James Coleman <jtc331@gmail.com>
Cc: pgsql-hackers <pgsql-hackers@postgresql.org>
Date: 2023-12-22T19:48:29Z
Lists: pgsql-hackers
Attachments
- WIP-refactor-predicate_implied_by_simple_clause.patch (text/x-diff) patch
James Coleman <jtc331@gmail.com> writes: > I've not yet applied all of your feedback, but I wanted to get an > initial read on your thoughts on how using switch statements ends up > looking. Attached is a single (pure refactor) patch that converts the > various if/else levels that check things like node tag and > boolean/null test type into switch statements. I've retained 'default' > keyword usages for now for simplicity (my intuition is that we > generally prefer to list out all options for compiler safety benefits, > though I'm not 100% sure that's useful in the outer node tag check > since it's unlikely that someone adding a new node would modify > this...). > My big question is: are you comfortable with the indentation explosion > this creates? IMO it's a lot wordier, but it is also more obvious what > the structural goal is. I'm not sure how we want to make the right > trade-off though. Yeah, I see what you mean. Also, I'd wanted to shove most of the text in the function header in-line and get rid of the short restatements of those paras. I carried that through just for predicate_implied_by_simple_clause, as attached. The structure is definitely clearer, but we end up with an awful lot of indentation, which makes the comments less readable than I'd like. (I did some minor rewording to make them flow better.) On balance I think this is probably better than what we have, but maybe we'd be best off to avoid doubly nested switches? I think there's a good argument for the outer switch on nodeTag, but maybe we're getting diminishing returns from an inner switch. regards, tom lane
Commits
-
Refactor predicate_{implied,refuted}_by_simple_clause.
- c7076ba6ad1c 17.0 landed