Re: [PATCHES] TODO item: list prepared queries
On Tue, 2006-01-03 at 18:00 -0500, Neil Conway wrote: > Anyway, if there was a reasonably cheap way to present the query strings > of protocol-level and SQL prepared statements in the same manner, I > think we should definitely do so. Since there doesn't appear to be one, > I'm content to just use the query string as sent by the user. I'll post > a revised patch that does that soon. Attached is the patch I applied to HEAD that uses the query string supplied by the client, without any rewriting. -Neil *** doc/src/sgml/catalogs.sgml 29fbade056e00ee4a48ba6a3f686627f62a103cf --- doc/src/sgml/catalogs.sgml f265188b9a381a6c2e67f94290b56f9dc993f5d1 *** *** 4373,4378 --- 4373,4383 + pg_prepared_statements + current prepared statements + + + pg_prepared_xacts currently prepared transactions *** *** 4778,4783 --- 4783,4883 + + pg_prepared_statements + + +pg_prepared_statements + + + +The pg_prepared_statements view displays +all the prepared statements that are available in the current +session. See for more information about prepared +statements. + + + +pg_prepared_statements contains one row +for each prepared statement. Rows are added to the view when a new +prepared statement is created, and removed when a prepared +statement is released (for example, via the +command). + + + +pg_prepared_statements Columns + + + + + Name + Type + References + Description + + + + + name + text + + +The identifier of the prepared statement. + + + + statement + text + + +The query string submitted by the client to create this +prepared statement. For prepared statements created via SQL, +this is the PREPARE statement submitted by +the client. For prepared statements created via the +frontend/backend protocol, this is the text of the prepared +statement itself. + + + + prepare_time + timestamptz + + +The time at which the prepared statement was created. + + + + parameter_types + oid[] + + +The expected parameter types for the prepared statement in the form of +an array of type OIDs. + + + + from_sql + boolean + + +true if the prepared statement was created +via the PREPARE SQL statement; +false if the statement was prepared via the +frontend/backend protocol. + + + + + + + +The pg_prepared_statements view is read only. + + + pg_prepared_xacts *** doc/src/sgml/ref/prepare.sgml 17fce269c43549b6ffa7bf3d5770da9fbdf18896 --- doc/src/sgml/ref/prepare.sgml 98824b3ad9ac4ffa50677f3b8ac821f8a1c84e16 *** *** 145,150 --- 145,155 the documentation. + + +You can see all available prepared statements of a session by querying the +pg_prepared_statements system view. + *** src/backend/catalog/system_views.sql 307260ff7bc30a48c0c60a40d4130f70310ebff2 --- src/backend/catalog/system_views.sql 7b92550bbcaf9d0ee99c12527f7785385cfeefe7 *** *** 156,161 --- 156,167 LEFT JOIN pg_authid U ON P.ownerid = U.oid LEFT JOIN pg_database D ON P.dbid = D.oid; + CREATE VIEW pg_prepared_statements AS + SELECT P.name, P.statement, P.prepare_time, P.parameter_types, P.from_sql + FROM pg_prepared_statement() AS P + (name text, statement text, prepare_time timestamptz, + parameter_types oid[], from_sql boolean); + CREATE VIEW pg_settings AS SELECT * FROM pg_show_all_settings() AS A *** src/backend/commands/prepare.c 3c1a8b677a84566407472a0b7b85b4cb86587956 --- src/backend/commands/prepare.c dc237de8d42ca2cd72d75e3b9bf64b8786702e5a *** *** 16,30 */ #include "postgres.h" #include "commands/explain.h" #include "commands/prepare.h" #include "executor/executor.h" ! #include "utils/guc.h" #include "optimizer/planner.h" #include "rewrite/rewriteHandler.h" #include "tcop/pquery.h" #include "tcop/tcopprot.h" #include "tcop/utility.h" #include "utils/hsearch.h" #include "utils/memutils.h" --- 16,35 */ #include "postgres.h" + #include "access/heapam.h" + #include "catalog/pg_type.h" #include "commands/explain.h" #include "commands/prepare.h" #include "executor/executor.h" ! #include "funcapi.h" ! #include "parser
Re: [PATCHES] psql tab completion enhancements
A few minor stylistic gripes...
On Fri, 2006-01-06 at 20:27 +0100, Joachim Wieland wrote:
> *** 150,155
> --- 151,171
> do {completion_charp = Query_for_list_of_attributes;
> completion_info_charp = table; matches = completion_matches(text,
> complete_from_query); } while(0)
>
> /*
> + * Keep the "malloced" keyword in all the names such that we
> remember that
> + * memory got allocated here. COMPLETE_WITH_MALLOCED_LIST frees this
> memory.
> + */
> + #define COMPLETE_WITH_MALLOCED_LIST(list) \
> + do { COMPLETE_WITH_LIST((const char**) list); free(list); list
> = (char**) 0; } while(0)
NULL is better style than 0 in a pointer context. Also, why are the
casts necessary?
> /* Forward declaration of functions */
> + static char **get_empty_list();
Should be "static char **get_empty_list(void);", as C89 doesn't check
the parameters passed to a function declared with an empty parameter
list.
> *** 754,760
> else if (pg_strcasecmp(prev3_wd, "TABLE") == 0 &&
> (pg_strcasecmp(prev_wd, "ALTER") == 0 ||
> pg_strcasecmp(prev_wd, "RENAME") == 0))
> ! COMPLETE_WITH_ATTR(prev2_wd);
>
> /* ALTER TABLE xxx RENAME yyy */
> else if (pg_strcasecmp(prev4_wd, "TABLE") == 0 &&
> --- 780,796
> else if (pg_strcasecmp(prev3_wd, "TABLE") == 0 &&
> (pg_strcasecmp(prev_wd, "ALTER") == 0 ||
> pg_strcasecmp(prev_wd, "RENAME") == 0))
> ! {
> ! char** list = GET_MALLOCED_LIST_WITH_ATTR(prev2_wd);
Should be "char **list = ..." -- similarly in several other places.
> *** 1454,1461
> else if (pg_strcasecmp(prev_wd, "LOCK") == 0 ||
> (pg_strcasecmp(prev_wd, "TABLE") == 0 &&
> pg_strcasecmp(prev2_wd, "LOCK") == 0))
> ! COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tables,
> NULL);
> !
> /* For the following, handle the case of a single table only
> for now */
>
> /* Complete LOCK [TABLE] with "IN" */
> --- 1497,1510
> else if (pg_strcasecmp(prev_wd, "LOCK") == 0 ||
> (pg_strcasecmp(prev_wd, "TABLE") == 0 &&
> pg_strcasecmp(prev2_wd, "LOCK") == 0))
> ! {
> ! char** list =
> GET_MALLOCED_LIST_WITH_SCHEMA_QUERY(Query_for_list_of_tables, NULL);
> ! /* if we have only seen LOCK but not LOCK TABLE so
> far, offer
> !* the TABLE keyword as well */
Is this actually useful? The "TABLE" keyword is just noise.
> *** 2315,2320
> --- 2368,2437
>
> }
>
> + /* LIST HELPER FUNCTIONS */
> +
> + static char**
> + get_empty_list() {
> + char** list = malloc(sizeof(char*));
> + list[0] = NULL;
> + return list;
> + }
Brace style ("{" on newline), "char **", and "(void"), as above.
> + static char**
> + get_query_list(const char *text,
> + const char *query,
> + const char* completion_info)
> + {
> + return _get_query_list(0, text, query, completion_info);
> + }
The difference between get_query_list() and _get_query_list() is not
reflected in the names of the methods. An "_internal" suffix would be
better, for example.
> + static char**
> + _get_query_list(int is_schema_query,
> + const char *text,
> + const char *query,
> + const char* completion_info)
"bool" for boolean variables rather than "int", and consistent parameter
declarations ("char *" not "char*").
> + static char**
> + list_add_item(char **list, char *item)
> + {
> + int size = 0;
> + while (list[size])
> + size++;
> + list = realloc(list, sizeof(char*) * (size + 1 + 1));
> + list[size] = item;
> + list[size+1] = NULL;
> + return list;
> + }
That's a terribly inefficient implementation.
-Neil
---(end of broadcast)---
TIP 4: Have you searched our list archives?
http://archives.postgresql.org
Re: [PATCHES] Summary table trigger example race condition
Mark Kirkwood wrote: Jim C. Nasby wrote: On Fri, Jan 06, 2006 at 02:00:34PM +1300, Mark Kirkwood wrote: However, I think the actual change is not quite right - after running DOH! It would be good if doc/src had a better mechanism for handling code; one that would allow for writing the code natively (so you don't have to worry about translating < into < and > into >) and for unit testing the different pieces of code. Yes it would - I usually build the SGML -> HTML, then cut the code out of a browser session to test - the pain is waiting for the docs to build. Anyway, updated patch attached. This one is good! After re-examining the original code, it looks like it was not actually vulnerable to a race condition! (it does the UPDATE, then if not found will do an INSERT, and handle unique violation with a repeat of the same UPDATE - i.e three DML statements, which are enough to handle the race in this case). However Jim's change handles the race needing only two DML statements in a loop, which seems much more elegant! In addition it provides a nice example of the 'merge' style code shown in e.g 36-1. Cheers Mark ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [PATCHES] [HACKERS] Inconsistent syntax in GRANT
On 1/7/06, Bruce Momjian wrote:
> Marko Kreen wrote:
> > The above table seem bit messy, but I see it as much easier to explain
> > to somebody.
>
> I am confused about your list above, so I can't see how that would be
> easy to explain.
Easy as in "use GRANT USAGE, forget about rest". You are confused
because you know the old way and look them together.
I would have liked to say "the rest are for fine-grained access control",
but with Tom's final proposal, the explanation would continue "SELECT,
UPDATE are for backwards compatibility".
Attached is a patch that fixes tablename->seqname and puts USAGE
as first in list to show it's the preferred way. I think it should
be mentioned somewhere explicitly, but I cant find proper place for
it. In the Compatibility section for GRANT?
--
marko
Index: pgsql/doc/src/sgml/ref/grant.sgml
===
*** pgsql.orig/doc/src/sgml/ref/grant.sgml
--- pgsql/doc/src/sgml/ref/grant.sgml
*** GRANT { { SELECT | INSERT | UPDATE | DEL
*** 25,33
ON [ TABLE ] tablename [, ...]
TO { username | GROUP groupname | PUBLIC } [, ...] [ WITH GRANT OPTION ]
! GRANT { { SELECT | USAGE | UPDATE }
[,...] | ALL [ PRIVILEGES ] }
! ON SEQUENCE tablename [, ...]
TO { username | GROUP groupname | PUBLIC } [, ...] [ WITH GRANT OPTION ]
GRANT { { CREATE | TEMPORARY | TEMP } [,...] | ALL [ PRIVILEGES ] }
--- 25,33
ON [ TABLE ] tablename [, ...]
TO { username | GROUP groupname | PUBLIC } [, ...] [ WITH GRANT OPTION ]
! GRANT { { USAGE | SELECT | UPDATE }
[,...] | ALL [ PRIVILEGES ] }
! ON SEQUENCE sequencename [, ...]
TO { username | GROUP groupname | PUBLIC } [, ...] [ WITH GRANT OPTION ]
GRANT { { CREATE | TEMPORARY | TEMP } [,...] | ALL [ PRIVILEGES ] }
Index: pgsql/doc/src/sgml/ref/revoke.sgml
===
*** pgsql.orig/doc/src/sgml/ref/revoke.sgml
--- pgsql/doc/src/sgml/ref/revoke.sgml
*** REVOKE [ GRANT OPTION FOR ]
*** 28,36
[ CASCADE | RESTRICT ]
REVOKE [ GRANT OPTION FOR ]
! { { SELECT | UPDATE }
[,...] | ALL [ PRIVILEGES ] }
! ON SEQUENCE tablename [, ...]
FROM { username | GROUP groupname | PUBLIC } [, ...]
[ CASCADE | RESTRICT ]
--- 28,36
[ CASCADE | RESTRICT ]
REVOKE [ GRANT OPTION FOR ]
! { { USAGE | SELECT | UPDATE }
[,...] | ALL [ PRIVILEGES ] }
! ON SEQUENCE sequencename [, ...]
FROM { username | GROUP groupname | PUBLIC } [, ...]
[ CASCADE | RESTRICT ]
---(end of broadcast)---
TIP 5: don't forget to increase your free space map settings
