Revues de code

Correctifs appliqués

Heikki Linnakangas a poussé :

Tom Lane a poussé :

  • Save a few cycles while creating "sticky" entries in pg_stat_statements. There's no need to sit there and increment the stats when we know all the increments would be zero anyway. The actual additions might not be very expensive, but skipping acquisition of the spinlock seems like a good thing. Pushing the logic about initialization of the usage count down into entry_alloc() allows us to do that while making the code actually simpler, not more complex. Expansion on a suggestion by Peter Geoghegan.
  • Don't bother copying empty support arrays in a zero-column MergeJoin. The case could not arise when this code was originally written, but it can now (since we made zero-column MergeJoins work for the benefit of FULL JOIN ON TRUE). I don't think there is any actual bug here, but we might as well treat it consistently with other uses of COPY_POINTER_FIELD(). Per comment from Ashutosh Bapat.
  • Fix an Assert that turns out to be reachable after all. estimate_num_groups() gets unhappy with create table empty(); select * from empty except select * from empty e2; I can't see any actual use-case for such a query (and the table is illegal per SQL spec), but it seems like a good idea that it not cause an assert failure.
  • Adjust various references to GEQO being non-deterministic. It's still non-deterministic in some sense ... but given fixed settings and identical planning problems, it will now always choose the same plan, so we probably shouldn't tar it with that brush. Per bug #6565 from Guillaume Cottenceau. Back-patch to 9.0 where the behavior was fixed.
  • Measure epoch of timestamp-without-time-zone from local not UTC midnight. This patch reverts commit 191ef2b407f065544ceed5700e42400857d9270f and thereby restores the pre-7.3 behavior of EXTRACT(EPOCH FROM timestamp-without-tz). Per discussion, the more recent behavior was misguided on a couple of grounds: it makes it hard to get a non-timezone-aware epoch value for a timestamp, and it makes this one case dependent on the value of the timezone GUC, which is incompatible with having timestamp_part() labeled as immutable. The other behavior is still available (in all releases) by explicitly casting the timestamp to timestamp with time zone before applying EXTRACT. This will need to be called out as an incompatible change in the 9.2 release notes. Although having mutable behavior in a function marked immutable is clearly a bug, we're not going to back-patch such a change.
  • Make pg_tablespace_location(0) return the database's default tablespace. This definition is convenient when applying the function to the reltablespace column of pg_class, since that's what zero means there; and it doesn't interfere with any other plausible use of the function. Per gripe from Bruce Momjian.
  • Silently ignore any nonexistent schemas that are listed in search_path. Previously we attempted to throw an error or at least warning for missing schemas, but this was done inconsistently because of implementation restrictions (in many cases, GUC settings are applied outside transactions so that we can't do system catalog lookups). Furthermore, there were exceptions to the rule even in the beginning, and we'd been poking more and more holes in it as time went on, because it turns out that there are lots of use-cases for having some irrelevant items in a common search_path value. It seems better to just adopt a philosophy similar to what's always been done with Unix PATH settings, wherein nonexistent or unreadable directories are silently ignored. This commit also fixes the documentation to point out that schemas for which the user lacks USAGE privilege are silently ignored. That's always been true but was previously not documented. This is mostly in response to Robert Haas' complaint that 9.1 started to throw errors or warnings for missing schemas in cases where prior releases had not. We won't adopt such a significant behavioral change in a back branch, so something different will be needed in 9.1.
  • Fix cost estimation for indexscan filter conditions. cost_index's method for estimating per-tuple costs of evaluating filter conditions (a/k/a qpquals) was completely wrong in the presence of derived indexable conditions, such as range conditions derived from a LIKE clause. This was largely masked in common cases as a result of all simple operator clauses having about the same costs, but it could show up in a big way when dealing with functional indexes containing expensive functions, as seen for example in bug #6579 from Istvan Endredy. Rejigger the calculation to give sane answers when the indexquals aren't a subset of the baserestrictinfo list. As a side benefit, we now do the calculation properly for cases involving join clauses (ie, parameterized indexscans), which we always overestimated before. There are still cases where this is an oversimplification, such as clauses that can be dropped because they are implied by a partial index's predicate. But we've never accounted for that in cost estimates before, and I'm not convinced it's worth the cycles to try to do so.
  • Assorted spelling corrections. Thom Brown
  • Remove the "last ditch" code path in join_search_one_level(). So far as I can tell, it is no longer possible for this heuristic to do anything useful, because the new weaker definition of have_relevant_joinclause means that any relation with a joinclause must be considered joinable to at least one other relation. It would still be possible for the code block to be entered, for example if there are join order restrictions that prevent any join of the current level from being formed; but in that case it's just a waste of cycles to attempt to form cartesian joins, since the restrictions will still apply. Furthermore, IMO the existence of this code path can mask bugs elsewhere; we would have noticed the problem with cartesian joins a lot sooner if this code hadn't compensated for it in the simplest case. Accordingly, let's remove it and see what happens. I'm committing this separately from the prerequisite changes in have_relevant_joinclause, just to make the question easier to revisit if there is some fault in my logic.
  • Weaken the planner's tests for relevant joinclauses. We should be willing to cross-join two small relations if that allows us to use an inner indexscan on a large relation (that is, the potential indexqual for the large table requires both smaller relations). This worked in simple cases but fell apart as soon as there was a join clause to a fourth relation, because the existence of any two-relation join clause caused the planner to not consider clauseless joins between other base relations. The added regression test shows an example case adapted from a recent complaint from Benoit Delbosc. Adjust have_relevant_joinclause, have_relevant_eclass_joinclause, and has_relevant_eclass_joinclause to consider that a join clause mentioning three or more relations is sufficient grounds for joining any subset of those relations, even if we have to do so via a cartesian join. Since such clauses are relatively uncommon, this shouldn't affect planning speed on typical queries; in fact it should help a bit, because the latter two functions in particular get significantly simpler. Although this is arguably a bug fix, I'm not going to risk back-patching it, since it might have currently-unforeseen consequences.

Bruce Momjian a poussé :

Peter Eisentraut a poussé :

Alvaro Herrera a poussé :

  • Accept postgres:// URIs in libpq connection functions. postgres:// URIs are an attempt to "stop the bleeding" in this general area that has been said to occur due to external projects adopting their own syntaxes. The syntaxes supported by this patch: postgres://[user[:pwd]@][unix-socket][:port[/dbname]][?param1=value1&...] postgres://[user[:pwd]@][net-location][:port][/dbname][?param1=value1&...] should be enough to cover most interesting cases without having to resort to "param=value" pairs, but those are provided for the cases that need them regardless. libpq documentation has been shuffled around a bit, to avoid stuffing all the format details into the PQconnectdbParams description, which was already a bit overwhelming. The list of keywords has moved to its own subsection, and the details on the URI format live in another subsection. This includes a simple test program, as requested in discussion, to ensure that interesting corner cases continue to work appropriately in the future. Author: Alexander Shulgin Some tweaking by Álvaro Herrera, Greg Smith, Daniel Farina, Peter Eisentraut Reviewed by Robert Haas, Alexey Klyukin (offlist), Heikki Linnakangas,Marko Kreen, and others Oh, it also supports postgresql:// but that's probably just an accident.

Robert Haas a poussé :

Tatsuo Ishii a poussé :

Correctifs rejetés (à ce jour)

  • Etsuro Fujita's patch to fix a typo in a comment.

Correctifs en attente

  • Vikash S. sent in a patch intended to allow psql to take multiple -f (file) arguments on the command line.
  • Robert Haas sent in another revision of the patch implementing pg_prewarm.
  • Robert Haas sent in another revision of a patch to fix a bug in fastpath locking.
  • Zoltan Boszormenyi sent in another revision of the patch for ECPG FETCH readahead.
  • Josh Kupershmidt sent in another revision of the patch to fix psql tab completion in the WITH case of CREATE/ALTER USER/ROLE.
  • Zoltan Boszormenyi sent in new versions of two patches: timeout framework and lock timeout.
  • Nikhil Sontakke sent in a patch implementing CHECK ONLY in CREATE/ALTER TABLE.
  • Heikki Linnakangas sent in a patch to allow compiling PL/pgsql-related items as EXTENSIONs.
  • Robert Haas sent in several patches to test different scenarios for memory usage during sorting.
  • Etsuro Fujita sent in a patch to add some validating machinery to FDWs and used same to check NULLness in the file_fdw.
  • Heikki Linnakangas sent in a patch to ensure that the right domain is used in errcontext() calls.
  • Tatsuo Ishii sent in a patch ensuring that pg_ctl --help actually documents all options the code makes available.