Re: [HACKERS] building pg_dump doesn't work
Dne 3.03.09 22:55, Tom Lane napsal(a):
One idea that comes to mind is to replace the entries like
{abort, ABORT_P, UNRESERVED_KEYWORD},
with macro calls
PG_KEYWORD(abort, ABORT_P, UNRESERVED_KEYWORD),
and then the frontend build of the file could define the macro
to ignore its second argument.
It sounds good.
The way we do it now seems to have other disadvantages too: we are
incorporating a backend .o file into pg_dump as-is, which would lead
to large problems if there were differences in say the compiler flags
needed. In fact, I thought Zdenek had been working on decoupling
that sort of thing, so I'm a bit surprised it's still like this at all.
Yeah, it is still on my TODO list. There is still problem with
pg_resetxlog which needs lot of internals headers. :(
Zdenek
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] building pg_dump doesn't work
Zdenek Kotala wrote:
Dne 3.03.09 22:55, Tom Lane napsal(a):
One idea that comes to mind is to replace the entries like
{abort, ABORT_P, UNRESERVED_KEYWORD},
with macro calls
PG_KEYWORD(abort, ABORT_P, UNRESERVED_KEYWORD),
and then the frontend build of the file could define the macro
to ignore its second argument.
It sounds good.
Please have a look at the patch I just posted -- should be up in a few
minutes in
http://archives.postgresql.org/message-id/20090305131208.GA4087%40alvh.no-ip.org
--
Alvaro Herrerahttp://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] building pg_dump doesn't work
Tom Lane wrote:
Alvaro Herrera alvhe...@commandprompt.com writes:
Seems doable.
Attached.
The TWO_MEMBER_SCANKEYWORD business seems a bit dangerous --- if the
header file is read without having #defined that correctly, bad things
will happen. It might be better to leave that out, always define the
struct the same, and just have pg_dump define PG_KEYWORD to fill the
value field with zero. Given alignment considerations, you're not
saving any space by omitting the field anyhow.
Fixed.
I also added #include type.h to the ecpg keywords.c file, which means we
don't need to redefine YYSTYPE at all on any of the three keywords.c
file. Looks cleaner overall.
Hopefully this is the last version of this patch.
--
Alvaro Herrerahttp://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support
Index: src/backend/parser/Makefile
===
RCS file: /home/alvherre/Code/cvs/pgsql/src/backend/parser/Makefile,v
retrieving revision 1.48
diff -c -p -r1.48 Makefile
*** src/backend/parser/Makefile 4 Oct 2008 21:56:54 - 1.48
--- src/backend/parser/Makefile 4 Mar 2009 15:32:52 -
*** override CPPFLAGS := -I$(srcdir) $(CPPFL
*** 14,20
OBJS= analyze.o gram.o keywords.o parser.o parse_agg.o parse_cte.o parse_clause.o \
parse_expr.o parse_func.o parse_node.o parse_oper.o parse_relation.o \
! parse_type.o parse_coerce.o parse_target.o parse_utilcmd.o scansup.o
FLEXFLAGS = -CF
--- 14,20
OBJS= analyze.o gram.o keywords.o parser.o parse_agg.o parse_cte.o parse_clause.o \
parse_expr.o parse_func.o parse_node.o parse_oper.o parse_relation.o \
! parse_type.o parse_coerce.o parse_target.o parse_utilcmd.o scansup.o kwlookup.o
FLEXFLAGS = -CF
Index: src/backend/parser/gram.y
===
RCS file: /home/alvherre/Code/cvs/pgsql/src/backend/parser/gram.y,v
retrieving revision 2.659
diff -c -p -r2.659 gram.y
*** src/backend/parser/gram.y 24 Feb 2009 10:06:33 - 2.659
--- src/backend/parser/gram.y 4 Mar 2009 17:04:39 -
*** static TypeName *TableFuncTypeName(List
*** 423,429
/*
* If you make any token changes, update the keyword table in
! * parser/keywords.c and add new keywords to the appropriate one of
* the reserved-or-not-so-reserved keyword lists, below; search
* this file for Name classification hierarchy.
*/
--- 423,429
/*
* If you make any token changes, update the keyword table in
! * src/include/parser/kwlist.h and add new keywords to the appropriate one of
* the reserved-or-not-so-reserved keyword lists, below; search
* this file for Name classification hierarchy.
*/
*** static TypeName *TableFuncTypeName(List
*** 516,522
ZONE
! /* The grammar thinks these are keywords, but they are not in the keywords.c
* list and so can never be entered directly. The filter in parser.c
* creates these tokens when required.
*/
--- 516,522
ZONE
! /* The grammar thinks these are keywords, but they are not in the kwlist.h
* list and so can never be entered directly. The filter in parser.c
* creates these tokens when required.
*/
Index: src/backend/parser/keywords.c
===
RCS file: /home/alvherre/Code/cvs/pgsql/src/backend/parser/keywords.c,v
retrieving revision 1.210
diff -c -p -r1.210 keywords.c
*** src/backend/parser/keywords.c 24 Feb 2009 10:06:33 - 1.210
--- src/backend/parser/keywords.c 4 Mar 2009 15:56:35 -
***
*** 3,10
* keywords.c
* lexical token lookup for key words in PostgreSQL
*
- * NB: This file is also used by pg_dump.
- *
*
* Portions Copyright (c) 1996-2009, PostgreSQL Global Development Group
* Portions Copyright (c) 1994, Regents of the University of California
--- 3,8
***
*** 15,503
*
*-
*/
! /* Use c.h so that this file can be built in either frontend or backend */
! #include c.h
!
! #include ctype.h
!
! /*
! * This macro definition overrides the YYSTYPE union definition in gram.h.
! * We don't need that struct in this file, and including the real definition
! * would require sucking in some backend-only include files.
! */
! #define YYSTYPE int
!
#include parser/keywords.h
- #ifndef ECPG_COMPILE
#include parser/gram.h
- #else
- #include preproc.h
- #endif
-
- /*
- * List of keyword (name, token-value, category) entries.
- *
- * !!WARNING!!: This list must be sorted by ASCII name, because binary
- * search is used to locate entries.
- */
- const ScanKeyword ScanKeywords[] = {
- /* name, value, category */
- {abort, ABORT_P, UNRESERVED_KEYWORD},
- {absolute, ABSOLUTE_P, UNRESERVED_KEYWORD},
-
Re: [HACKERS] building pg_dump doesn't work
On Thu, Mar 5, 2009 at 1:12 PM, Alvaro Herrera
alvhe...@commandprompt.com wrote:
Tom Lane wrote:
Alvaro Herrera alvhe...@commandprompt.com writes:
Seems doable.
Attached.
The TWO_MEMBER_SCANKEYWORD business seems a bit dangerous --- if the
header file is read without having #defined that correctly, bad things
will happen. It might be better to leave that out, always define the
struct the same, and just have pg_dump define PG_KEYWORD to fill the
value field with zero. Given alignment considerations, you're not
saving any space by omitting the field anyhow.
Fixed.
I also added #include type.h to the ecpg keywords.c file, which means we
don't need to redefine YYSTYPE at all on any of the three keywords.c
file. Looks cleaner overall.
Hopefully this is the last version of this patch.
FWIW gcc does this kind of trick all over the place. They have lists
of various types of objects, not unlike our nodes and define them in
.h files which contain _just_ the equivalent of PG_KEYWORD. Then they
include those files in various places with the macro defined to do
different things. So for example they define an enum, an array for
external consumption, an array for internal consumption, and in many
places even switch statements with trivial bits of code from the macro
too.
If we're going to go this route I think it does make sense to move the
const ScanKeyword ScanKeywords[] = { preamble and to live with the
PG_KEYWORD definition. Even if we're not planning to have any other
kinds of macro definitions aside from ScanKeywords arrays today it
would be nice to have the flexibility and in any case I don't really
like the action-at-a-distance of having a macro definition in one
place which depends on the definition in another place.
--
greg
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] building pg_dump doesn't work
Alvaro Herrera alvhe...@commandprompt.com writes: Hopefully this is the last version of this patch. A few more comments would help --- in particular the header comment for kwlist.h should explain that the calling file is supposed to define PG_KEYWORD appropriately for its needs. I also wonder whether Greg isn't right that it would be better if the header contained *only* the PG_KEYWORD macros, rather than presupposing that the caller wants to build a constant table named ScanKeywords with them. In general though it's certainly cleaner than the old way. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] building pg_dump doesn't work
Alvaro Herrera wrote:
Hmm, I had thought that pg_dump only wanted the header file, not the
keywords.o object file. I now see that I was wrong. I agree that your
proposed solution is a lot better. I'll see about it.
Here it is. The #ifdef parts seem a bit ugly, but I'm not sure how can
this be improved, given that ECPG is already using this file.
--
Alvaro Herrerahttp://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support
Index: src/backend/parser/keywords.c
===
RCS file: /home/alvherre/Code/cvs/pgsql/src/backend/parser/keywords.c,v
retrieving revision 1.210
diff -c -p -r1.210 keywords.c
*** src/backend/parser/keywords.c 24 Feb 2009 10:06:33 - 1.210
--- src/backend/parser/keywords.c 4 Mar 2009 14:34:53 -
***
*** 29,41
#define YYSTYPE int
#include parser/keywords.h
#ifndef ECPG_COMPILE
#include parser/gram.h
! #else
#include preproc.h
#endif
/*
* List of keyword (name, token-value, category) entries.
*
* !!WARNING!!: This list must be sorted by ASCII name, because binary
--- 29,54
#define YYSTYPE int
#include parser/keywords.h
+
#ifndef ECPG_COMPILE
+ #ifndef FRONTEND
#include parser/gram.h
! #endif
! #else /* ECPG_COMPILE */
#include preproc.h
#endif
/*
+ * We don't want to include the gram.h file on frontend builds, except ECPG, so
+ * leave out the second struct member in that case.
+ */
+ #if !defined FRONTEND || defined ECPG_COMPILE
+ #define PG_KEYWORD(a,b,c) {a,b,c}
+ #else
+ #define PG_KEYWORD(a,b,c) {a,c}
+ #endif
+
+ /*
* List of keyword (name, token-value, category) entries.
*
* !!WARNING!!: This list must be sorted by ASCII name, because binary
***
*** 43,439
*/
const ScanKeyword ScanKeywords[] = {
/* name, value, category */
! {abort, ABORT_P, UNRESERVED_KEYWORD},
! {absolute, ABSOLUTE_P, UNRESERVED_KEYWORD},
! {access, ACCESS, UNRESERVED_KEYWORD},
! {action, ACTION, UNRESERVED_KEYWORD},
! {add, ADD_P, UNRESERVED_KEYWORD},
! {admin, ADMIN, UNRESERVED_KEYWORD},
! {after, AFTER, UNRESERVED_KEYWORD},
! {aggregate, AGGREGATE, UNRESERVED_KEYWORD},
! {all, ALL, RESERVED_KEYWORD},
! {also, ALSO, UNRESERVED_KEYWORD},
! {alter, ALTER, UNRESERVED_KEYWORD},
! {always, ALWAYS, UNRESERVED_KEYWORD},
! {analyse, ANALYSE, RESERVED_KEYWORD}, /* British spelling */
! {analyze, ANALYZE, RESERVED_KEYWORD},
! {and, AND, RESERVED_KEYWORD},
! {any, ANY, RESERVED_KEYWORD},
! {array, ARRAY, RESERVED_KEYWORD},
! {as, AS, RESERVED_KEYWORD},
! {asc, ASC, RESERVED_KEYWORD},
! {assertion, ASSERTION, UNRESERVED_KEYWORD},
! {assignment, ASSIGNMENT, UNRESERVED_KEYWORD},
! {asymmetric, ASYMMETRIC, RESERVED_KEYWORD},
! {at, AT, UNRESERVED_KEYWORD},
! {authorization, AUTHORIZATION, TYPE_FUNC_NAME_KEYWORD},
! {backward, BACKWARD, UNRESERVED_KEYWORD},
! {before, BEFORE, UNRESERVED_KEYWORD},
! {begin, BEGIN_P, UNRESERVED_KEYWORD},
! {between, BETWEEN, TYPE_FUNC_NAME_KEYWORD},
! {bigint, BIGINT, COL_NAME_KEYWORD},
! {binary, BINARY, TYPE_FUNC_NAME_KEYWORD},
! {bit, BIT, COL_NAME_KEYWORD},
! {boolean, BOOLEAN_P, COL_NAME_KEYWORD},
! {both, BOTH, RESERVED_KEYWORD},
! {by, BY, UNRESERVED_KEYWORD},
! {cache, CACHE, UNRESERVED_KEYWORD},
! {called, CALLED, UNRESERVED_KEYWORD},
! {cascade, CASCADE, UNRESERVED_KEYWORD},
! {cascaded, CASCADED, UNRESERVED_KEYWORD},
! {case, CASE, RESERVED_KEYWORD},
! {cast, CAST, RESERVED_KEYWORD},
! {catalog, CATALOG_P, UNRESERVED_KEYWORD},
! {chain, CHAIN, UNRESERVED_KEYWORD},
! {char, CHAR_P, COL_NAME_KEYWORD},
! {character, CHARACTER, COL_NAME_KEYWORD},
! {characteristics, CHARACTERISTICS, UNRESERVED_KEYWORD},
! {check, CHECK, RESERVED_KEYWORD},
! {checkpoint, CHECKPOINT, UNRESERVED_KEYWORD},
! {class, CLASS, UNRESERVED_KEYWORD},
! {close, CLOSE, UNRESERVED_KEYWORD},
! {cluster, CLUSTER, UNRESERVED_KEYWORD},
! {coalesce, COALESCE, COL_NAME_KEYWORD},
! {collate, COLLATE, RESERVED_KEYWORD},
! {column, COLUMN, RESERVED_KEYWORD},
! {comment, COMMENT, UNRESERVED_KEYWORD},
! {commit, COMMIT, UNRESERVED_KEYWORD},
! {committed, COMMITTED, UNRESERVED_KEYWORD},
! {concurrently, CONCURRENTLY, UNRESERVED_KEYWORD},
! {configuration, CONFIGURATION, UNRESERVED_KEYWORD},
! {connection, CONNECTION, UNRESERVED_KEYWORD},
! {constraint, CONSTRAINT, RESERVED_KEYWORD},
! {constraints, CONSTRAINTS, UNRESERVED_KEYWORD},
! {content, CONTENT_P, UNRESERVED_KEYWORD},
! {continue, CONTINUE_P, UNRESERVED_KEYWORD},
! {conversion, CONVERSION_P, UNRESERVED_KEYWORD},
! {copy, COPY, UNRESERVED_KEYWORD},
! {cost, COST, UNRESERVED_KEYWORD},
! {create, CREATE, RESERVED_KEYWORD},
! {createdb, CREATEDB, UNRESERVED_KEYWORD},
! {createrole, CREATEROLE, UNRESERVED_KEYWORD},
! {createuser, CREATEUSER, UNRESERVED_KEYWORD},
! {cross, CROSS, TYPE_FUNC_NAME_KEYWORD},
! {csv, CSV, UNRESERVED_KEYWORD},
! {ctype, CTYPE,
Re: [HACKERS] building pg_dump doesn't work
Heikki Linnakangas heikki.linnakan...@enterprisedb.com writes: Perhaps this could be made less ugly by only having the ScanKeywords array in the .c file, and #including that into other .c files in src/backend/parser, ecpg and pg_dump. What I'd suggest is something similar to the design of the errcodes.h header: create a header file containing just the list of PG_KEYWORD macro calls, and have the different users #include it after defining that macro appropriately. Having .c files include other .c files is usually best avoided on least-surprise grounds. Not sure what to do about ScanKeywordLookup function. Yeah, duplicating that function is a bit annoying. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] building pg_dump doesn't work
Alvaro Herrera wrote:
Alvaro Herrera wrote:
Hmm, I had thought that pg_dump only wanted the header file, not the
keywords.o object file. I now see that I was wrong. I agree that your
proposed solution is a lot better. I'll see about it.
Here it is. The #ifdef parts seem a bit ugly, but I'm not sure how can
this be improved, given that ECPG is already using this file.
Perhaps this could be made less ugly by only having the ScanKeywords
array in the .c file, and #including that into other .c files in
src/backend/parser, ecpg and pg_dump.
So, keywords.c would look like:
#include parser/keywords.h
const ScanKeyword ScanKeywords[] = {
/* name, value, category */
PG_KEYWORD(abort, ABORT_P, UNRESERVED_KEYWORD),
PG_KEYWORD(absolute, ABSOLUTE_P, UNRESERVED_KEYWORD),
PG_KEYWORD(access, ACCESS, UNRESERVED_KEYWORD),
...
And there would be a new file in src/bin/pg_dump, say dumpkeywords.c,
that looks like this:
#include c.h
#define PG_KEYWORD(a,b,c) {a,b,c}
#include src/backend/parser/keywords.c
Not sure what to do about ScanKeywordLookup function.
/*
+ * We don't want to include the gram.h file on frontend builds, except ECPG,
so
+ * leave out the second struct member in that case.
+ */
+ #if !defined FRONTEND || defined ECPG_COMPILE
+ #define PG_KEYWORD(a,b,c) {a,b,c}
+ #else
+ #define PG_KEYWORD(a,b,c) {a,c}
+ #endif
Doesn't that put 'c' into the wrong field in ScanKeyword struct? It only
compiles because both 'value' and 'category' are int16.
--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] building pg_dump doesn't work
Tom Lane wrote: Heikki Linnakangas heikki.linnakan...@enterprisedb.com writes: Perhaps this could be made less ugly by only having the ScanKeywords array in the .c file, and #including that into other .c files in src/backend/parser, ecpg and pg_dump. What I'd suggest is something similar to the design of the errcodes.h header: create a header file containing just the list of PG_KEYWORD macro calls, and have the different users #include it after defining that macro appropriately. Having .c files include other .c files is usually best avoided on least-surprise grounds. Seems doable. Not sure what to do about ScanKeywordLookup function. Yeah, duplicating that function is a bit annoying. Another new file, backend/parser/kwlookup.c perhaps? -- Alvaro Herrerahttp://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] building pg_dump doesn't work
Heikki Linnakangas wrote:
Alvaro Herrera wrote:
/*
+ * We don't want to include the gram.h file on frontend builds, except
ECPG, so
+ * leave out the second struct member in that case.
+ */
+ #if !defined FRONTEND || defined ECPG_COMPILE
+ #define PG_KEYWORD(a,b,c) {a,b,c}
+ #else
+ #define PG_KEYWORD(a,b,c) {a,c}
+ #endif
Doesn't that put 'c' into the wrong field in ScanKeyword struct? It only
compiles because both 'value' and 'category' are int16.
No, because I had the header with the second field omitted too.
--
Alvaro Herrerahttp://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] building pg_dump doesn't work
Alvaro Herrera wrote:
Tom Lane wrote:
What I'd suggest is something similar to the design of the errcodes.h
header: create a header file containing just the list of PG_KEYWORD
macro calls, and have the different users #include it after defining
that macro appropriately. Having .c files include other .c files is
usually best avoided on least-surprise grounds.
Seems doable.
Attached.
--
Alvaro Herrerahttp://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.
Index: src/backend/parser/Makefile
===
RCS file: /home/alvherre/Code/cvs/pgsql/src/backend/parser/Makefile,v
retrieving revision 1.48
diff -c -p -r1.48 Makefile
*** src/backend/parser/Makefile 4 Oct 2008 21:56:54 - 1.48
--- src/backend/parser/Makefile 4 Mar 2009 15:32:52 -
*** override CPPFLAGS := -I$(srcdir) $(CPPFL
*** 14,20
OBJS= analyze.o gram.o keywords.o parser.o parse_agg.o parse_cte.o parse_clause.o \
parse_expr.o parse_func.o parse_node.o parse_oper.o parse_relation.o \
! parse_type.o parse_coerce.o parse_target.o parse_utilcmd.o scansup.o
FLEXFLAGS = -CF
--- 14,20
OBJS= analyze.o gram.o keywords.o parser.o parse_agg.o parse_cte.o parse_clause.o \
parse_expr.o parse_func.o parse_node.o parse_oper.o parse_relation.o \
! parse_type.o parse_coerce.o parse_target.o parse_utilcmd.o scansup.o kwlookup.o
FLEXFLAGS = -CF
Index: src/backend/parser/keywords.c
===
RCS file: /home/alvherre/Code/cvs/pgsql/src/backend/parser/keywords.c,v
retrieving revision 1.210
diff -c -p -r1.210 keywords.c
*** src/backend/parser/keywords.c 24 Feb 2009 10:06:33 - 1.210
--- src/backend/parser/keywords.c 4 Mar 2009 15:56:35 -
***
*** 3,10
* keywords.c
* lexical token lookup for key words in PostgreSQL
*
- * NB: This file is also used by pg_dump.
- *
*
* Portions Copyright (c) 1996-2009, PostgreSQL Global Development Group
* Portions Copyright (c) 1994, Regents of the University of California
--- 3,8
***
*** 15,503
*
*-
*/
! /* Use c.h so that this file can be built in either frontend or backend */
! #include c.h
!
! #include ctype.h
!
! /*
! * This macro definition overrides the YYSTYPE union definition in gram.h.
! * We don't need that struct in this file, and including the real definition
! * would require sucking in some backend-only include files.
! */
! #define YYSTYPE int
!
#include parser/keywords.h
- #ifndef ECPG_COMPILE
#include parser/gram.h
- #else
- #include preproc.h
- #endif
-
- /*
- * List of keyword (name, token-value, category) entries.
- *
- * !!WARNING!!: This list must be sorted by ASCII name, because binary
- * search is used to locate entries.
- */
- const ScanKeyword ScanKeywords[] = {
- /* name, value, category */
- {abort, ABORT_P, UNRESERVED_KEYWORD},
- {absolute, ABSOLUTE_P, UNRESERVED_KEYWORD},
- {access, ACCESS, UNRESERVED_KEYWORD},
- {action, ACTION, UNRESERVED_KEYWORD},
- {add, ADD_P, UNRESERVED_KEYWORD},
- {admin, ADMIN, UNRESERVED_KEYWORD},
- {after, AFTER, UNRESERVED_KEYWORD},
- {aggregate, AGGREGATE, UNRESERVED_KEYWORD},
- {all, ALL, RESERVED_KEYWORD},
- {also, ALSO, UNRESERVED_KEYWORD},
- {alter, ALTER, UNRESERVED_KEYWORD},
- {always, ALWAYS, UNRESERVED_KEYWORD},
- {analyse, ANALYSE, RESERVED_KEYWORD}, /* British spelling */
- {analyze, ANALYZE, RESERVED_KEYWORD},
- {and, AND, RESERVED_KEYWORD},
- {any, ANY, RESERVED_KEYWORD},
- {array, ARRAY, RESERVED_KEYWORD},
- {as, AS, RESERVED_KEYWORD},
- {asc, ASC, RESERVED_KEYWORD},
- {assertion, ASSERTION, UNRESERVED_KEYWORD},
- {assignment, ASSIGNMENT, UNRESERVED_KEYWORD},
- {asymmetric, ASYMMETRIC, RESERVED_KEYWORD},
- {at, AT, UNRESERVED_KEYWORD},
- {authorization, AUTHORIZATION, TYPE_FUNC_NAME_KEYWORD},
- {backward, BACKWARD, UNRESERVED_KEYWORD},
- {before, BEFORE, UNRESERVED_KEYWORD},
- {begin, BEGIN_P, UNRESERVED_KEYWORD},
- {between, BETWEEN, TYPE_FUNC_NAME_KEYWORD},
- {bigint, BIGINT, COL_NAME_KEYWORD},
- {binary, BINARY, TYPE_FUNC_NAME_KEYWORD},
- {bit, BIT, COL_NAME_KEYWORD},
- {boolean, BOOLEAN_P, COL_NAME_KEYWORD},
- {both, BOTH, RESERVED_KEYWORD},
- {by, BY, UNRESERVED_KEYWORD},
- {cache, CACHE, UNRESERVED_KEYWORD},
- {called, CALLED, UNRESERVED_KEYWORD},
- {cascade, CASCADE, UNRESERVED_KEYWORD},
- {cascaded, CASCADED, UNRESERVED_KEYWORD},
- {case, CASE, RESERVED_KEYWORD},
- {cast, CAST, RESERVED_KEYWORD},
- {catalog, CATALOG_P, UNRESERVED_KEYWORD},
- {chain, CHAIN, UNRESERVED_KEYWORD},
- {char, CHAR_P, COL_NAME_KEYWORD},
- {character, CHARACTER, COL_NAME_KEYWORD},
- {characteristics, CHARACTERISTICS, UNRESERVED_KEYWORD},
- {check, CHECK, RESERVED_KEYWORD},
- {checkpoint, CHECKPOINT,
Re: [HACKERS] building pg_dump doesn't work
Alvaro Herrera wrote:
Alvaro Herrera wrote:
Tom Lane wrote:
What I'd suggest is something similar to the design of the errcodes.h
header: create a header file containing just the list of PG_KEYWORD
macro calls, and have the different users #include it after defining
that macro appropriately. Having .c files include other .c files is
usually best avoided on least-surprise grounds.
Seems doable.
Attached.
Minor fixes over the previous patch.
--
Alvaro Herrerahttp://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support
diff -u src/bin/pg_dump/Makefile src/bin/pg_dump/Makefile
--- src/bin/pg_dump/Makefile4 Mar 2009 15:25:11 -
+++ src/bin/pg_dump/Makefile4 Mar 2009 17:02:19 -
@@ -27,13 +27,13 @@
all: submake-libpq submake-libpgport pg_dump pg_restore pg_dumpall
-pg_dump: pg_dump.o common.o pg_dump_sort.o $(OBJS) $(libpq_builddir)/libpq.a
+pg_dump: pg_dump.o common.o pg_dump_sort.o $(OBJS) $(KEYWRDOBJS)
$(libpq_builddir)/libpq.a
$(CC) $(CFLAGS) pg_dump.o common.o pg_dump_sort.o $(KEYWRDOBJS) $(OBJS)
$(libpq_pgport) $(LDFLAGS) $(LIBS) -o $...@$(X)
-pg_restore: pg_restore.o $(OBJS) $(libpq_builddir)/libpq.a
+pg_restore: pg_restore.o $(OBJS) $(KEYWRDOBJS) $(libpq_builddir)/libpq.a
$(CC) $(CFLAGS) pg_restore.o $(KEYWRDOBJS) $(OBJS) $(libpq_pgport)
$(LDFLAGS) $(LIBS) -o $...@$(X)
-pg_dumpall: pg_dumpall.o dumputils.o $(libpq_builddir)/libpq.a
+pg_dumpall: pg_dumpall.o dumputils.o $(KEYWRDOBJS) $(libpq_builddir)/libpq.a
$(CC) $(CFLAGS) pg_dumpall.o dumputils.o $(KEYWRDOBJS) $(WIN32RES)
$(libpq_pgport) $(LDFLAGS) $(LIBS) -o $...@$(X)
install: all installdirs
diff -u src/bin/pg_dump/keywords.c src/bin/pg_dump/keywords.c
--- src/bin/pg_dump/keywords.c 4 Mar 2009 16:08:22 -
+++ src/bin/pg_dump/keywords.c 4 Mar 2009 17:03:08 -
@@ -15,18 +15,11 @@
*/
#include postgres_fe.h
-/*
- * This macro definition overrides the YYSTYPE union definition in gram.h.
- * We don't need that struct in this file, and including the real definition
- * would require sucking in some backend-only include files.
- */
-#define YYSTYPE int
-
#include parser/keywords.h
/*
- * We don't need the token number, so leave it out to avoid requiring extra
- * object files from the backend.
+ * We don't need the token number, so leave it out to avoid requiring other
+ * backend headers.
*/
#define PG_KEYWORD(a,b,c) {a,c}
#define TWO_MEMBER_SCANKEYWORD
only in patch2:
unchanged:
--- src/backend/parser/gram.y 24 Feb 2009 10:06:33 - 2.659
+++ src/backend/parser/gram.y 4 Mar 2009 17:04:39 -
@@ -423,7 +423,7 @@ static TypeName *TableFuncTypeName(List
/*
* If you make any token changes, update the keyword table in
- * parser/keywords.c and add new keywords to the appropriate one of
+ * src/include/parser/kwlist.h and add new keywords to the appropriate one of
* the reserved-or-not-so-reserved keyword lists, below; search
* this file for Name classification hierarchy.
*/
@@ -516,7 +516,7 @@ static TypeName *TableFuncTypeName(List
ZONE
-/* The grammar thinks these are keywords, but they are not in the keywords.c
+/* The grammar thinks these are keywords, but they are not in the kwlist.h
* list and so can never be entered directly. The filter in parser.c
* creates these tokens when required.
*/
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] building pg_dump doesn't work
Alvaro Herrera alvhe...@commandprompt.com writes: Seems doable. Attached. The TWO_MEMBER_SCANKEYWORD business seems a bit dangerous --- if the header file is read without having #defined that correctly, bad things will happen. It might be better to leave that out, always define the struct the same, and just have pg_dump define PG_KEYWORD to fill the value field with zero. Given alignment considerations, you're not saving any space by omitting the field anyhow. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] building pg_dump doesn't work
Hi, I noticed that if you start from a clean tree, it doesn't work to build pg_dump because gram.h has not been generated yet: make -C ../../../src/backend/parser keywords.o make[1]: Entering directory `/home/alvherre/Code/CVS/pgsql/build/00head/src/backend/parser' gcc -O0 -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement -Wendif-labels -fno-strict-aliasing -fwrapv -g -I/pgsql/source/00head/src/backend/parser -I../../../src/include -I/pgsql/source/00head/src/include -D_GNU_SOURCE -I/usr/include/libxml2 -c -o keywords.o /pgsql/source/00head/src/backend/parser/keywords.c -MMD -MP -MF .deps/keywords.Po /pgsql/source/00head/src/backend/parser/keywords.c:33:25: error: parser/gram.h: No such file or directory /pgsql/source/00head/src/backend/parser/keywords.c:46: error: 'ABORT_P' undeclared here (not in a function) I notice that there's a line in backend/parser that makes keywords.o depend on gram.h, but apparently it doesn't work: # Force these dependencies to be known even without dependency info built: gram.o keywords.o parser.o: $(srcdir)/gram.h The pg_dump Makefile appears to be expecting the file to be in src/include/parser. This works for src/backend/parser because it adds the current srcdir as -I to CPPFLAGS. (Actually I see, and vaguely remember, that this has been broken for a long time.) -- Alvaro Herrerahttp://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] building pg_dump doesn't work
Alvaro Herrera wrote: Hi, I noticed that if you start from a clean tree, it doesn't work to build pg_dump because gram.h has not been generated yet: make -C ../../../src/backend/parser keywords.o make[1]: Entering directory `/home/alvherre/Code/CVS/pgsql/build/00head/src/backend/parser' gcc -O0 -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement -Wendif-labels -fno-strict-aliasing -fwrapv -g -I/pgsql/source/00head/src/backend/parser -I../../../src/include -I/pgsql/source/00head/src/include -D_GNU_SOURCE -I/usr/include/libxml2 -c -o keywords.o /pgsql/source/00head/src/backend/parser/keywords.c -MMD -MP -MF .deps/keywords.Po /pgsql/source/00head/src/backend/parser/keywords.c:33:25: error: parser/gram.h: No such file or directory /pgsql/source/00head/src/backend/parser/keywords.c:46: error: 'ABORT_P' undeclared here (not in a function) This patch fixes it. The fmgroids.h bits are not needed for pg_dump, but they are if you start building on the parser directory. I don't care strongly for that one, so if people don't like it I'll just go ahead with the gram.h bit, unless anyone has a better idea. -- Alvaro Herrerahttp://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support Index: src/backend/parser/Makefile === RCS file: /home/alvherre/Code/cvs/pgsql/src/backend/parser/Makefile,v retrieving revision 1.48 diff -c -p -r1.48 Makefile *** src/backend/parser/Makefile 4 Oct 2008 21:56:54 - 1.48 --- src/backend/parser/Makefile 3 Mar 2009 21:18:17 - *** subdir = src/backend/parser *** 10,17 top_builddir = ../../.. include $(top_builddir)/src/Makefile.global - override CPPFLAGS := -I$(srcdir) $(CPPFLAGS) - OBJS= analyze.o gram.o keywords.o parser.o parse_agg.o parse_cte.o parse_clause.o \ parse_expr.o parse_func.o parse_node.o parse_oper.o parse_relation.o \ parse_type.o parse_coerce.o parse_target.o parse_utilcmd.o scansup.o --- 10,15 *** endif *** 52,57 --- 50,63 # Force these dependencies to be known even without dependency info built: gram.o keywords.o parser.o: $(srcdir)/gram.h + keywords.o: $(top_builddir)/src/include/parser/gram.h + parse_func.o: $(top_builddir)/src/include/utils/fmgroids.h + parse_coerce.o: $(top_builddir)/src/include/utils/fmgroids.h + + $(top_builddir)/src/include/parser/gram.h: + $(MAKE) -C $(top_builddir)/src/backend ../../src/include/parser/gram.h + $(top_builddir)/src/include/utils/fmgroids.h: + $(MAKE) -C $(top_builddir)/src/backend ../../src/include/utils/fmgroids.h # gram.c, gram.h, and scan.c are in the distribution tarball, so they -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] building pg_dump doesn't work
Alvaro Herrera alvhe...@commandprompt.com writes:
Alvaro Herrera wrote:
I noticed that if you start from a clean tree, it doesn't work to build
pg_dump because gram.h has not been generated yet:
This patch fixes it.
I think this is probably going in the wrong direction. The reason
gram.h isn't already in the main include tree is that we don't *want*
all and sundry depending on it --- we have very carefully minimized
the number of files that depend on the grammar's symbol codes.
ISTM that pg_dump doesn't actually care about the symbol codes, it
just needs a list of known keywords. Can we refactor things so that
the frontend-side version of the keyword list doesn't include the
grammar symbols at all?
One idea that comes to mind is to replace the entries like
{abort, ABORT_P, UNRESERVED_KEYWORD},
with macro calls
PG_KEYWORD(abort, ABORT_P, UNRESERVED_KEYWORD),
and then the frontend build of the file could define the macro
to ignore its second argument.
The way we do it now seems to have other disadvantages too: we are
incorporating a backend .o file into pg_dump as-is, which would lead
to large problems if there were differences in say the compiler flags
needed. In fact, I thought Zdenek had been working on decoupling
that sort of thing, so I'm a bit surprised it's still like this at all.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] building pg_dump doesn't work
Tom Lane wrote: I think this is probably going in the wrong direction. The reason gram.h isn't already in the main include tree is that we don't *want* all and sundry depending on it --- we have very carefully minimized the number of files that depend on the grammar's symbol codes. ISTM that pg_dump doesn't actually care about the symbol codes, it just needs a list of known keywords. Hmm, I had thought that pg_dump only wanted the header file, not the keywords.o object file. I now see that I was wrong. I agree that your proposed solution is a lot better. I'll see about it. -- Alvaro Herrerahttp://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
