On Wed, Nov 13, 2024 at 09:00:30PM +0100, Tomas Vondra wrote: > Does that mean you think we should fix the issue at hand differently? > Say, by looking at number of columns and building the correct tuple, > like I did in my initial patch?
691e8b2e18 is not something I would have done when it comes to pageinspect, FWIW. There is the superuser argument for this module, so I'd vote for an error and apply the same policy across all branches as a matter of consistency. A second argument in favor of raising an error is that it makes future changes a bit easier to implement so as you can reuse the existing routine as an "internal" workhorse called by the SQL functions depending on the version of the extension involved at runtime, as we do in pgss. Data type changes are easier to track, for example. If you add a solution based on the number of arguments, you may have to mix it with a per-version approach, at least on HEAD. As long as there is a test to check on behavior or another, we're going to be fine because we'll know if a previous assumption suddenly breaks. Anyway, your solution to use the number of columns to determine what should be done at runtime works as well, so feel free to just ignore me. That's just an approach I'd prefer taking here because that's easier to work with IMO, but I can also see why people don't like the versioning logic a-la-PGSS, as well. -- Michael
signature.asc
Description: PGP signature
