summaryrefslogtreecommitdiff
path: root/src/backend/parser/parse_utilcmd.c
AgeCommit message (Collapse)Author
2020-12-01Ensure that expandTableLikeClause() re-examines the same table.Tom Lane
As it stood, expandTableLikeClause() re-did the same relation_openrv call that transformTableLikeClause() had done. However there are scenarios where this would not find the same table as expected. We hold lock on the LIKE source table, so it can't be renamed or dropped, but another table could appear before it in the search path. This explains the odd behavior reported in bug #16758 when cloning a table as a temp table of the same name. This case worked as expected before commit 502898192 introduced the need to open the source table twice, so we should fix it. To make really sure we get the same table, let's re-open it by OID not name. That requires adding an OID field to struct TableLikeClause, which is a little nervous-making from an ABI standpoint, but as long as it's at the end I don't think there's any serious risk. Per bug #16758 from Marc Boeren. Like the previous patch, back-patch to all supported branches. Discussion: https://postgr.es/m/16758-840e84a6cfab276d@postgresql.org
2020-11-19Further fixes for CREATE TABLE LIKE: cope with self-referential FKs.Tom Lane
Commit 502898192 was too careless about the order of execution of the additional ALTER TABLE operations generated by expandTableLikeClause. It just stuck them all at the end, which seems okay for most purposes. But it falls down in the case where LIKE is importing a primary key or unique index and the outer CREATE TABLE includes a FOREIGN KEY constraint that needs to depend on that index. Weird as that is, it used to work, so we ought to keep it working. To fix, make parse_utilcmd.c insert LIKE clauses between index-creation and FK-creation commands in the transformed list of commands, and change utility.c so that the commands generated by expandTableLikeClause are executed immediately not at the end. One could imagine scenarios where this wouldn't work either; but currently expandTableLikeClause only makes column default expressions, CHECK constraints, and indexes, and this ordering seems fine for those. Per bug #16730 from Sofoklis Papasofokli. Like the previous patch, back-patch to all supported branches. Discussion: https://postgr.es/m/16730-b902f7e6e0276b30@postgresql.org
2020-08-21Fix handling of CREATE TABLE LIKE with inheritance.Tom Lane
If a CREATE TABLE command uses both LIKE and traditional inheritance, Vars in CHECK constraints and expression indexes that are absorbed from a LIKE parent table tended to get mis-numbered, resulting in wrong answers and/or bizarre error messages (though probably not any actual crashes, thanks to validation occurring in the executor). In v12 and up, the same could happen to Vars in GENERATED expressions, even in cases with no LIKE clause but multiple traditional-inheritance parents. The cause of the problem for LIKE is that parse_utilcmd.c supposed it could renumber such Vars correctly during transformCreateStmt(), which it cannot since we have not yet accounted for columns added via inheritance. Fix that by postponing processing of LIKE INCLUDING CONSTRAINTS, DEFAULTS, GENERATED, INDEXES till after we've performed DefineRelation(). The error with GENERATED and multiple inheritance is a simple oversight in MergeAttributes(); it knows it has to renumber Vars in inherited CHECK constraints, but forgot to apply the same processing to inherited GENERATED expressions (a/k/a defaults). Per bug #16272 from Tom Gottfried. The non-GENERATED variants of the issue are ancient, presumably dating right back to the addition of CREATE TABLE LIKE; hence back-patch to all supported branches. Discussion: https://postgr.es/m/16272-6e32da020e9a9381@postgresql.org
2020-03-22Revert "Skip WAL for new relfilenodes, under wal_level=minimal."Noah Misch
This reverts commit cb2fd7eac285b1b0a24eeb2b8ed4456b66c5a09f. Per numerous buildfarm members, it was incompatible with parallel query, and a test case assumed LP64. Back-patch to 9.5 (all supported versions). Discussion: https://postgr.es/m/20200321224920.GB1763544@rfd.leadboat.com
2020-03-21Skip WAL for new relfilenodes, under wal_level=minimal.Noah Misch
Until now, only selected bulk operations (e.g. COPY) did this. If a given relfilenode received both a WAL-skipping COPY and a WAL-logged operation (e.g. INSERT), recovery could lose tuples from the COPY. See src/backend/access/transam/README section "Skipping WAL for New RelFileNode" for the new coding rules. Maintainers of table access methods should examine that section. To maintain data durability, just before commit, we choose between an fsync of the relfilenode and copying its contents to WAL. A new GUC, wal_skip_threshold, guides that choice. If this change slows a workload that creates small, permanent relfilenodes under wal_level=minimal, try adjusting wal_skip_threshold. Users setting a timeout on COMMIT may need to adjust that timeout, and log_min_duration_statement analysis will reflect time consumption moving to COMMIT from commands like COPY. Internally, this requires a reliable determination of whether RollbackAndReleaseCurrentSubTransaction() would unlink a relation's current relfilenode. Introduce rd_firstRelfilenodeSubid. Amend the specification of rd_createSubid such that the field is zero when a new rel has an old rd_node. Make relcache.c retain entries for certain dropped relations until end of transaction. Back-patch to 9.5 (all supported versions). This introduces a new WAL record type, XLOG_GIST_ASSIGN_LSN, without bumping XLOG_PAGE_MAGIC. As always, update standby systems before master systems. This changes sizeof(RelationData) and sizeof(IndexStmt), breaking binary compatibility for affected extensions. (The most recent commit to affect the same class of extensions was 089e4d405d0f3b94c74a2c6a54357a84a681754b.) Kyotaro Horiguchi, reviewed (in earlier, similar versions) by Robert Haas. Heikki Linnakangas and Michael Paquier implemented earlier designs that materially clarified the problem. Reviewed, in earlier designs, by Andrew Dunstan, Andres Freund, Alvaro Herrera, Tom Lane, Fujii Masao, and Simon Riggs. Reported by Martijn van Oosterhout. Discussion: https://postgr.es/m/20150702220524.GA9392@svana.org
2017-08-03Allow a foreign table CHECK constraint to be initially NOT VALID.Robert Haas
For a table, the constraint can be considered validated immediately, because the table must be empty. But for a foreign table this is not necessarily the case. Fixes a bug in commit f27a6b15e6566fba7748d0d9a3fc5bcfd52c4a1b. Amit Langote, with some changes by me. Discussion: http://postgr.es/m/d2b7419f-4a71-cf86-cc99-bfd0f359a1ea@lab.ntt.co.jp
2016-12-22Fix CREATE TABLE ... LIKE ... WITH OIDS.Tom Lane
Having a WITH OIDS specification should result in the creation of an OID column, but commit b943f502b broke that in the case that there were LIKE tables without OIDS. Commentary in that patch makes it look like this was intentional, but if so it was based on a faulty reading of what inheritance does: the parent tables can add an OID column, but they can't subtract one. AFAICS, the behavior ought to be that you get an OID column if any of the inherited tables, LIKE tables, or WITH clause ask for one. Also, revert that patch's unnecessary split of transformCreateStmt's loop over the tableElts list into two passes. That seems to have been based on a misunderstanding as well: we already have two-pass processing here, we don't need three passes. Per bug #14474 from Jeff Dafoe. Back-patch to 9.6 where the misbehavior was introduced. Report: https://postgr.es/m/20161222145304.25620.47445@wrigleys.postgresql.org
2016-07-28Improve documentation about CREATE TABLE ... LIKE.Tom Lane
The docs failed to explain that LIKE INCLUDING INDEXES would not preserve the names of indexes and associated constraints. Also, it wasn't mentioned that EXCLUDE constraints would be copied by this option. The latter oversight seems enough of a documentation bug to justify back-patching. In passing, do some minor copy-editing in the same area, and add an entry for LIKE under "Compatibility", since it's not exactly a faithful implementation of the standard's feature. Discussion: <20160728151154.AABE64016B@smtp.hushmail.com>
2016-06-09pgindent run for 9.6Robert Haas
2016-04-08Revert CREATE INDEX ... INCLUDING ...Teodor Sigaev
It's not ready yet, revert two commits 690c543550b0d2852060c18d270cdb534d339d9a - unstable test output 386e3d7609c49505e079c40c65919d99feb82505 - patch itself
2016-04-08CREATE INDEX ... INCLUDING (column[, ...])Teodor Sigaev
Now indexes (but only B-tree for now) can contain "extra" column(s) which doesn't participate in index structure, they are just stored in leaf tuples. It allows to use index only scan by using single index instead of two or more indexes. Author: Anastasia Lubennikova with minor editorializing by me Reviewers: David Rowley, Peter Geoghegan, Jeff Janes
2016-03-23Support CREATE ACCESS METHODAlvaro Herrera
This enables external code to create access methods. This is useful so that extensions can add their own access methods which can be formally tracked for dependencies, so that DROP operates correctly. Also, having explicit support makes pg_dump work correctly. Currently only index AMs are supported, but we expect different types to be added in the future. Authors: Alexander Korotkov, Petr Jelínek Reviewed-By: Teodor Sigaev, Petr Jelínek, Jim Nasby Commitfest-URL: https://commitfest.postgresql.org/9/353/ Discussion: https://www.postgresql.org/message-id/CAPpHfdsXwZmojm6Dx+TJnpYk27kT4o7Ri6X_4OSWcByu1Rm+VA@mail.gmail.com
2016-02-11Move pg_constraint.h function declarations to new file pg_constraint_fn.h.Tom Lane
A pending patch requires exporting a function returning Bitmapset from catalog/pg_constraint.c. As things stand, that would mean including nodes/bitmapset.h in pg_constraint.h, which might be hazardous for the client-side includability of that header. It's not entirely clear whether any client-side code needs to include pg_constraint.h, but it seems prudent to assume that there is some such code somewhere. Therefore, split off the function definitions into a new file pg_constraint_fn.h, similarly to what we've done for some other catalog header files.
2016-01-17Restructure index access method API to hide most of it at the C level.Tom Lane
This patch reduces pg_am to just two columns, a name and a handler function. All the data formerly obtained from pg_am is now provided in a C struct returned by the handler function. This is similar to the designs we've adopted for FDWs and tablesample methods. There are multiple advantages. For one, the index AM's support functions are now simple C functions, making them faster to call and much less error-prone, since the C compiler can now check function signatures. For another, this will make it far more practical to define index access methods in installable extensions. A disadvantage is that SQL-level code can no longer see attributes of index AMs; in particular, some of the crosschecks in the opr_sanity regression test are no longer possible from SQL. We've addressed that by adding a facility for the index AM to perform such checks instead. (Much more could be done in that line, but for now we're content if the amvalidate functions more or less replace what opr_sanity used to do.) We might also want to expose some sort of reporting functionality, but this patch doesn't do that. Alexander Korotkov, reviewed by Petr Jelínek, and rather heavily editorialized on by me.
2016-01-02Update copyright for 2016Bruce Momjian
Backpatch certain files through 9.1
2015-12-16Mark CHECK constraints declared NOT VALID valid if created with table.Robert Haas
FOREIGN KEY constraints have behaved this way for a long time, but for some reason the behavior of CHECK constraints has been inconsistent up until now. Amit Langote and Amul Sul, with assorted tweaks by me.
2015-10-22Fix typos in comments.Robert Haas
CharSyam
2015-10-05Have CREATE TABLE LIKE add OID column if any LIKEd table has oneBruce Momjian
Also, process constraints for LIKEd tables at the end so an OID column can be referenced in a constraint. Report by Tom Lane
2015-05-23pgindent run for 9.5Bruce Momjian
2015-04-28Fix another test for RELKIND_RELATION that should allow foreign tables now.Tom Lane
I thought I'd gone through all of these before, but a fresh review found this one too. (Perhaps it would be better to just delete this test and let the failure occur later, but for the moment I'll preserve the logic.) The case that this was rejecting is like CREATE FOREIGN TABLE ft (f1 int ...) ...; CREATE TABLE c1 (UNIQUE(f1)) INHERITS(ft);
2015-04-25Add comments warning against generalizing default_with_oids.Tom Lane
pg_dump has historically assumed that default_with_oids affects only plain tables and not other relkinds. Conceivably we could make it apply to some newly invented relkind if we did so from the get-go, but changing the behavior for existing object types will break existing dump scripts. Add code comments warning about this interaction. Also, make sure that default_with_oids doesn't cause parse_utilcmd.c to think that CREATE FOREIGN TABLE will create an OID column. I think this is only a latent bug right now, since we don't allow UNIQUE/PKEY constraints in CREATE FOREIGN TABLE, but it's better to be consistent and future-proof.
2015-04-25Revert: Honor OID status of CREATE LIKE'd tablesBruce Momjian
Reverts d992f8a8961c09ec219373ffe2b5e6473febd065 Report by Tom Lane
2015-04-20Honor OID status of CREATE LIKE'd tablesBruce Momjian
Previously, tables created by CREATE LIKE never had OIDs. Report by Tom Lane
2015-04-03Transform ALTER TABLE/SET TYPE/USING expr during parse analysisAlvaro Herrera
This lets later stages have access to the transformed expression; in particular it allows DDL-deparsing code during event triggers to pass the transformed expression to ruleutils.c, so that the complete command can be deparsed. This shuffles the timing of the transform calls a bit: previously, nothing was transformed during parse analysis, and only the RELKIND_RELATION case was being handled during execution. After this patch, all expressions are transformed during parse analysis (including those for relkinds other than RELATION), and the error for other relation kinds is thrown only during execution. So we do more work than before to reject some bogus cases. That seems acceptable.
2015-03-18Setup cursor position for schema-qualified elementsAlvaro Herrera
This makes any errors thrown while looking up such schemas report the position of the error. Author: Ryan Kelly Reviewed by: Jeevan Chalke, Tom Lane
2015-03-09Allow CURRENT/SESSION_USER to be used in certain commandsAlvaro Herrera
Commands such as ALTER USER, ALTER GROUP, ALTER ROLE, GRANT, and the various ALTER OBJECT / OWNER TO, as well as ad-hoc clauses related to roles such as the AUTHORIZATION clause of CREATE SCHEMA, the FOR clause of CREATE USER MAPPING, and the FOR ROLE clause of ALTER DEFAULT PRIVILEGES can now take the keywords CURRENT_USER and SESSION_USER as user specifiers in place of an explicit user name. This commit also fixes some quite ugly handling of special standards- mandated syntax in CREATE USER MAPPING, which in particular would fail to work in presence of a role named "current_user". The special role specifiers PUBLIC and NONE also have more consistent handling now. Also take the opportunity to add location tracking to user specifiers. Authors: Kyotaro Horiguchi. Heavily reworked by Álvaro Herrera. Reviewed by: Rushabh Lathia, Adam Brightwell, Marti Raudsepp.
2015-02-22Get rid of multiple applications of transformExpr() to the same tree.Tom Lane
transformExpr() has for many years had provisions to do nothing when applied to an already-transformed expression tree. However, this was always ugly and of dubious reliability, so we'd be much better off without it. The primary historical reason for it was that gram.y sometimes returned multiple links to the same subexpression, which is no longer true as of my BETWEEN fixes. We'd also grown some lazy hacks in CREATE TABLE LIKE (failing to distinguish between raw and already-transformed index specifications) and one or two other places. This patch removes the need for and support for re-transforming already transformed expressions. The index case is dealt with by adding a flag to struct IndexStmt to indicate that it's already been transformed; which has some benefit anyway in that tablecmds.c can now Assert that transformation has happened rather than just assuming. The other main reason was some rather sloppy code for array type coercion, which can be fixed (and its performance improved too) by refactoring. I did leave transformJoinUsingClause() still constructing expressions containing untransformed operator nodes being applied to Vars, so that transformExpr() still has to allow Var inputs. But that's a much narrower, and safer, special case than before, since Vars will never appear in a raw parse tree, and they don't have any substructure to worry about. In passing fix some oversights in the patch that added CREATE INDEX IF NOT EXISTS (missing processing of IndexStmt.if_not_exists). These appear relatively harmless, but still sloppy coding practice.
2015-02-03Fix typo in comment.Heikki Linnakangas
Amit Langote
2015-01-06Update copyright for 2015Bruce Momjian
Backpatch certain files through 9.0
2014-12-23get_object_address: separate domain constraints from table constraintsAlvaro Herrera
Apart from enabling comments on domain constraints, this enables a future project to replicate object dropping to remote servers: with the current mechanism there's no way to distinguish between the two types of constraints, so there's no way to know what to drop. Also added support for the domain constraint comments in psql's \dd and pg_dump. Catalog version bumped due to the change in ObjectType enum.
2014-12-17Allow CHECK constraints to be placed on foreign tables.Tom Lane
As with NOT NULL constraints, we consider that such constraints are merely reports of constraints that are being enforced by the remote server (or other underlying storage mechanism). Their only real use is to allow planner optimizations, for example in constraint-exclusion checks. Thus, the code changes here amount to little more than removal of the error that was formerly thrown for applying CHECK to a foreign table. (In passing, do a bit of cleanup of the ALTER FOREIGN TABLE reference page, which had accumulated some weird decisions about ordering etc.) Shigeru Hanada and Etsuro Fujita, reviewed by Kyotaro Horiguchi and Ashutosh Bapat.
2014-05-06pgindent run for 9.4Bruce Momjian
This includes removing tabs after periods in C comments, which was applied to back branches, so this change should not effect backpatching.
2014-02-17Avoid repeated name lookups during table and index DDL.Robert Haas
If the name lookups come to different conclusions due to concurrent activity, we might perform some parts of the DDL on a different table than other parts. At least in the case of CREATE INDEX, this can be used to cause the permissions checks to be performed against a different table than the index creation, allowing for a privilege escalation attack. This changes the calling convention for DefineIndex, CreateTrigger, transformIndexStmt, transformAlterTableStmt, CheckIndexCompatible (in 9.2 and newer), and AlterTable (in 9.1 and older). In addition, CheckRelationOwnership is removed in 9.2 and newer and the calling convention is changed in older branches. A field has also been added to the Constraint node (FkConstraint in 8.4). Third-party code calling these functions or using the Constraint node will require updating. Report by Andres Freund. Patch by Robert Haas and Andres Freund, reviewed by Tom Lane. Security: CVE-2014-0062
2014-01-07Update copyright for 2014Bruce Momjian
Update all files in head, and files COPYRIGHT and legal.sgml in all back branches.
2013-11-21Support multi-argument UNNEST(), and TABLE() syntax for multiple functions.Tom Lane
This patch adds the ability to write TABLE( function1(), function2(), ...) as a single FROM-clause entry. The result is the concatenation of the first row from each function, followed by the second row from each function, etc; with NULLs inserted if any function produces fewer rows than others. This is believed to be a much more useful behavior than what Postgres currently does with multiple SRFs in a SELECT list. This syntax also provides a reasonable way to combine use of column definition lists with WITH ORDINALITY: put the column definition list inside TABLE(), where it's clear that it doesn't control the ordinality column as well. Also implement SQL-compliant multiple-argument UNNEST(), by turning UNNEST(a,b,c) into TABLE(unnest(a), unnest(b), unnest(c)). The SQL standard specifies TABLE() with only a single function, not multiple functions, and it seems to require an implicit UNNEST() which is not what this patch does. There may be something wrong with that reading of the spec, though, because if it's right then the spec's TABLE() is just a pointless alternative spelling of UNNEST(). After further review of that, we might choose to adopt a different syntax for what this patch does, but in any case this functionality seems clearly worthwhile. Andrew Gierth, reviewed by Zoltán Böszörményi and Heikki Linnakangas, and significantly revised by me
2013-08-07Message style improvementsPeter Eisentraut
2013-07-05Update messages, comments and documentation for materialized views.Noah Misch
All instances of the verbiage lagging the code. Back-patch to 9.3, where materialized views were introduced.
2013-07-01Add a convenience routine makeFuncCall to reduce duplication.Robert Haas
David Fetter and Andrew Gierth, reviewed by Jeevan Chalke
2013-05-29pgindent run for release 9.3Bruce Momjian
This is the first run of the Perl-based pgindent script. Also update pgindent instructions.
2013-04-20Clean up references to SQL92Peter Eisentraut
In most cases, these were just references to the SQL standard in general. In a few cases, a contrast was made between SQL92 and later standards -- those have been kept unchanged.
2013-04-12Clean up the mess around EXPLAIN and materialized views.Tom Lane
Revert the matview-related changes in explain.c's API, as per recent complaint from Robert Haas. The reason for these appears to have been principally some ill-considered choices around having intorel_startup do what ought to be parse-time checking, plus a poor arrangement for passing it the view parsetree it needs to store into pg_rewrite when creating a materialized view. Do the latter by having parse analysis stick a copy into the IntoClause, instead of doing it at runtime. (On the whole, I seriously question the choice to represent CREATE MATERIALIZED VIEW as a variant of SELECT INTO/CREATE TABLE AS, because that means injecting even more complexity into what was already a horrid legacy kluge. However, I didn't go so far as to rethink that choice ... yet.) I also moved several error checks into matview parse analysis, and made the check for external Params in a matview more accurate. In passing, clean things up a bit more around interpretOidsOption(), and fix things so that we can use that to force no-oids for views, sequences, etc, thereby eliminating the need to cons up "oids = false" options when creating them. catversion bump due to change in IntoClause. (I wonder though if we really need readfuncs/outfuncs support for IntoClause anymore.)
2013-03-22Fix problems with incomplete attempt to prohibit OIDS with MVs.Kevin Grittner
Problem with assertion failure in restoring from pg_dump output reported by Joachim Wieland. Review and suggestions by Tom Lane and Robert Haas.
2013-03-12Allow default expressions to be attached to columns of foreign tables.Tom Lane
There's still some discussion about exactly how postgres_fdw ought to handle this case, but there seems no debate that we want to allow defaults to be used for inserts into foreign tables. So remove the core-code restrictions that prevented it. While at it, get rid of the special grammar productions for CREATE FOREIGN TABLE, and instead add explicit FEATURE_NOT_SUPPORTED error checks for the disallowed cases. This makes the grammar a shade smaller, and more importantly results in much more intelligible error messages for unsupported cases. It's also one less thing to fix if we ever start supporting constraints on foreign tables.
2013-03-03Add a materialized view relations.Kevin Grittner
A materialized view has a rule just like a view and a heap and other physical properties like a table. The rule is only used to populate the table, references in queries refer to the materialized data. This is a minimal implementation, but should still be useful in many cases. Currently data is only populated "on demand" by the CREATE MATERIALIZED VIEW and REFRESH MATERIALIZED VIEW statements. It is expected that future releases will add incremental updates with various timings, and that a more refined concept of defining what is "fresh" data will be developed. At some point it may even be possible to have queries use a materialized in place of references to underlying tables, but that requires the other above-mentioned features to be working first. Much of the documentation work by Robert Haas. Review by Noah Misch, Thom Brown, Robert Haas, Marko Tiikkaja Security review by KaiGai Kohei, with a decision on how best to implement sepgsql still pending.
2013-01-01Update copyrights for 2013Bruce Momjian
Fully update git head, and update back branches in ./COPYRIGHT and legal.sgml files.
2012-11-28Fix assorted bugs in CREATE/DROP INDEX CONCURRENTLY.Tom Lane
Commit 8cb53654dbdb4c386369eb988062d0bbb6de725e, which introduced DROP INDEX CONCURRENTLY, managed to break CREATE INDEX CONCURRENTLY via a poor choice of catalog state representation. The pg_index state for an index that's reached the final pre-drop stage was the same as the state for an index just created by CREATE INDEX CONCURRENTLY. This meant that the (necessary) change to make RelationGetIndexList ignore about-to-die indexes also made it ignore freshly-created indexes; which is catastrophic because the latter do need to be considered in HOT-safety decisions. Failure to do so leads to incorrect index entries and subsequently wrong results from queries depending on the concurrently-created index. To fix, add an additional boolean column "indislive" to pg_index, so that the freshly-created and about-to-die states can be distinguished. (This change obviously is only possible in HEAD. This patch will need to be back-patched, but in 9.2 we'll use a kluge consisting of overloading the formerly-impossible state of indisvalid = true and indisready = false.) In addition, change CREATE/DROP INDEX CONCURRENTLY so that the pg_index flag changes they make without exclusive lock on the index are made via heap_inplace_update() rather than a normal transactional update. The latter is not very safe because moving the pg_index tuple could result in concurrent SnapshotNow scans finding it twice or not at all, thus possibly resulting in index corruption. This is a pre-existing bug in CREATE INDEX CONCURRENTLY, which was copied into the DROP code. In addition, fix various places in the code that ought to check to make sure that the indexes they are manipulating are valid and/or ready as appropriate. These represent bugs that have existed since 8.2, since a failed CREATE INDEX CONCURRENTLY could leave a corrupt or invalid index behind, and we ought not try to do anything that might fail with such an index. Also fix RelationReloadIndexInfo to ensure it copies all the pg_index columns that are allowed to change after initial creation. Previously we could have been left with stale values of some fields in an index relcache entry. It's not clear whether this actually had any user-visible consequences, but it's at least a bug waiting to happen. In addition, do some code and docs review for DROP INDEX CONCURRENTLY; some cosmetic code cleanup but mostly addition and revision of comments. This will need to be back-patched, but in a noticeably different form, so I'm committing it to HEAD before working on the back-patch. Problem reported by Amit Kapila, diagnosis by Pavan Deolassee, fix by Tom Lane and Andres Freund.
2012-08-30Split tuple struct defs from htup.h to htup_details.hAlvaro Herrera
This reduces unnecessary exposure of other headers through htup.h, which is very widely included by many files. I have chosen to move the function prototypes to the new file as well, because that means htup.h no longer needs to include tupdesc.h. In itself this doesn't have much effect in indirect inclusion of tupdesc.h throughout the tree, because it's also required by execnodes.h; but it's something to explore in the future, and it seemed best to do the htup.h change now while I'm busy with it.
2012-08-10Centralize the logic for detecting misplaced aggregates, window funcs, etc.Tom Lane
Formerly we relied on checking after-the-fact to see if an expression contained aggregates, window functions, or sub-selects when it shouldn't. This is grotty, easily forgotten (indeed, we had forgotten to teach DefineIndex about rejecting window functions), and none too efficient since it requires extra traversals of the parse tree. To improve matters, define an enum type that classifies all SQL sub-expressions, store it in ParseState to show what kind of expression we are currently parsing, and make transformAggregateCall, transformWindowFuncCall, and transformSubLink check the expression type and throw error if the type indicates the construct is disallowed. This allows removal of a large number of ad-hoc checks scattered around the code base. The enum type is sufficiently fine-grained that we can still produce error messages of at least the same specificity as before. Bringing these error checks together revealed that we'd been none too consistent about phrasing of the error messages, so standardize the wording a bit. Also, rewrite checking of aggregate arguments so that it requires only one traversal of the arguments, rather than up to three as before. In passing, clean up some more comments left over from add_missing_from support, and annotate some tests that I think are dead code now that that's gone. (I didn't risk actually removing said dead code, though.)
2012-07-16Avoid pre-determining index names during CREATE TABLE LIKE parsing.Tom Lane
Formerly, when trying to copy both indexes and comments, CREATE TABLE LIKE had to pre-assign names to indexes that had comments, because it made up an explicit CommentStmt command to apply the comment and so it had to know the name for the index. This creates bad interactions with other indexes, as shown in bug #6734 from Daniele Varrazzo: the preassignment logic couldn't take any other indexes into account so it could choose a conflicting name. To fix, add a field to IndexStmt that allows it to carry a comment to be assigned to the new index. (This isn't a user-exposed feature of CREATE INDEX, only an internal option.) Now we don't need preassignment of index names in any situation. I also took the opportunity to refactor DefineIndex to accept the IndexStmt as such, rather than passing all its fields individually in a mile-long parameter list. Back-patch to 9.2, but no further, because it seems too dangerous to change IndexStmt or DefineIndex's API in released branches. The bug exists back to 9.0 where CREATE TABLE LIKE grew the ability to copy comments, but given the lack of prior complaints we'll just let it go unfixed before 9.2.
2012-07-04Reduce messages about implicit indexes and sequences to DEBUG1.Robert Haas
Per recent discussion on pgsql-hackers, these messages are too chatty for most users.