[HACKERS] Reporting planning time with EXPLAIN
Hi, We report planning and execution time when EXPLAIN ANALYZE is issued. We do not have facility to report planning time as part EXPLAIN output. In order to get the planning time, one has to issue EXPLAIN ANALYZE which involves executing the plan, which is unnecessary. We report planning and execution times when es->summary is true. It is set to true when es->analyze is true. 211 /* currently, summary option is not exposed to users; just set it */ 212 es->summary = es->analyze; The comment was introduced by commit commit 90063a7612e2730f7757c2a80ba384bbe7e35c4b Author: Tom Lane Date: Wed Oct 15 18:50:13 2014 -0400 Print planning time only in EXPLAIN ANALYZE, not plain EXPLAIN. We've gotten enough push-back on that change to make it clear that it wasn't an especially good idea to do it like that. Revert plain EXPLAIN to its previous behavior, but keep the extra output in EXPLAIN ANALYZE. Per discussion. Internally, I set this up as a separate flag ExplainState.summary that controls printing of planning time and execution time. For now it's just copied from the ANALYZE option, but we could consider exposing it to users. The discussion referred to seems to be [1]. Here's patch to expose the "summary" option as mentioned in the last paragraph of above commit message. Right now I have named it as "summary", but I am fine if we want to change it to something meaningful. "timing" already has got some other usage, so can't use it here. One can use this option as postgres=# explain (summary on) select * from pg_class c, pg_type t where c.reltype = t.oid; QUERY PLAN -- Hash Join (cost=17.12..35.70 rows=319 width=511) Hash Cond: (c.reltype = t.oid) -> Seq Scan on pg_class c (cost=0.00..14.19 rows=319 width=259) -> Hash (cost=12.61..12.61 rows=361 width=256) -> Seq Scan on pg_type t (cost=0.00..12.61 rows=361 width=256) Planning time: 48.823 ms (6 rows) When analyze is specified, summary is also set to ON. By default this flag is OFF. Suggestions welcome. [1] https://www.postgresql.org/message-id/flat/1351f76f-69a4-4257-91c2-9382e2a6dc22%40email.android.com#1351f76f-69a4-4257-91c2-9382e2a6d...@email.android.com [2] https://www.postgresql.org/message-id/19766.1413129321%40sss.pgh.pa.us -- Best Wishes, Ashutosh Bapat EnterpriseDB Corporation The Postgres Database Company pg_explain_plan_time.patch Description: binary/octet-stream -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] merging some features from plpgsql2 project
Hi I reread ideas described on page https://github.com/trustly/plpgsql2 Some points are well and can be benefit for PlpgSQL. First I describe my initial position. I am strongly against introduction "new" language - plpgsql2 or new plpgsql, or any else. The trust of developers to us is important and introduction of any not compatible or different feature has to have really big reason. PostgreSQL is conservative environment, and PLpgSQL should not be a exception. More - I have not any information from my customers, colleagues about missing features in this language. If there is some gaps, then it is in outer environment - IDE, deployment, testing, I understand so we have to do some break compatibility changes, but the changes should be smoothly with possibility to simply identify necessary changes, but better don't do it - and use other possibility. i lost hope so plpgsql_check can be integrated to core. It is living outside well - only preparing dll for MSWindows is annoyance. But some checks from plpgsql_check can be implemented in core as extra_checks, and some checks from plpgsql2 can be implemented in plpgsql_check. Points from plpgsql2: * SELECT .. INTO vs. TOO_MANY_ROWS - can be implemented as extra check * SELECT .. INTO and the number of result columns - good extra check too * EXECUTE and FOUND - this is incompatible change, extra check can be used (test on unset variable). I see solution in leaving FOUND variable and introduction of some new without this issue - ROW_COUNT maybe (this is another possible incompatible change, but with higher benefit - maybe we can introduce some aliasing, PRAGMA clause, default PRAGMAs, ..). * SELECT .. INTO and := - looks bizarre, but I see clean benefit and I can accept it * The OUT namespace and OUT parameter visibility - I don't like it - not in this form - can we introduce some form of namespace aliasing? The arguments are in function name named namespace already. Now, we doesn't use plpgsql_extra_check much and it is pity. I thing so real question is support some macros, that can help with code maintenance for different PostgreSQL versions. PostGIS code is nice example what we are missing. Regards Pavel
Re: [HACKERS] postgres_fdw bug in 9.6
On 2016/12/22 1:04, Ashutosh Bapat wrote: Some review comments Thanks for the review! 2. We should try to look for other not-so-cheap paths if the cheapest one is paramterized. You might want to use get_cheapest_path_for_pathkeys() to find a suitable unparameterized path by passing NULL for required_outer and NIL for pathkeys, that's a very strange usage, but I think it will serve the purpose. On the thread we discussed that we should save the epq_path create for lower join and use it here. That may be better than searching for a path. +/* Give up if the cheapest-total-cost paths are parameterized. */ +if (!bms_is_empty(PATH_REQ_OUTER(outer_path)) || +!bms_is_empty(PATH_REQ_OUTER(inner_path))) +return NULL; I did that because I think that would work well for postgres_fdw, but I agree with you. Will revise. 3. Talking about saving some CPU cycles - if a clauseless full join can be implemented only using merge join, probably that's the only path available in the list of paths for the given relation. Instead of building the same path anew, should we use the existing path like GetExistingLocalJoinPath() for that case? Hm, that might be an idea, but my concern about that is: the existing path wouldn't always be guaranteed to be unprameterized. In fact, I am wondering whether we should look for an existing nestloop path for all joins except full, in which case we should look for merge or hash join path. We go on building a new path, if only there isn't an existing one. That will certainly save some CPU cycles spent in costing the path. Maybe in many cases, nestloop paths for INNER/LEFT/SEMI/ANTI might be removed from the rel's pathlist by dominated merge or hash join paths, so searching the pathlist might cause a useless overhead. 4. Following comment mentions only hash join, but the code creates a merge join or hash join path. * If the jointype is JOIN_FULL, try to create a hashjoin join path from Will revise. 5. Per comment below a clauseless full join can be implemented using only merge join. Why are we checking extra->mergejoin_allowed? Shouldn't it be true here? /* * Special corner case: for "x FULL JOIN y ON true", there will be no * join clauses at all. Note that mergejoin is our only join type * that supports FULL JOIN without any join clauses, and in that case * it doesn't require the input paths to be well ordered, so generate * a clauseless mergejoin path from the cheapest-total-cost paths. */ if (extra->mergejoin_allowed && !extra->mergeclause_list) See the comments for select_mergejoin_clauses: * select_mergejoin_clauses *Select mergejoin clauses that are usable for a particular join. *Returns a list of RestrictInfo nodes for those clauses. * * *mergejoin_allowed is normally set to TRUE, but it is set to FALSE if * this is a right/full join and there are nonmergejoinable join clauses. * The executor's mergejoin machinery cannot handle such cases, so we have * to avoid generating a mergejoin plan. (Note that this flag does NOT * consider whether there are actually any mergejoinable clauses. This is * correct because in some cases we need to build a clauseless mergejoin. * Simply returning NIL is therefore not enough to distinguish safe from * unsafe cases.) Rethinking about the problem, the error comes because the inner or outer plan of a merge join plan doesn't have pathkeys required by the merge join. This happens because the merge join path had foreign path with pathkeys as inner or outer path and corresponding fdw_outerpath didn't have those pathkeys. I am wondering whether the easy and possibly correct solution here is to not replace a ForeignPath with fdw_outerpath in GetExistingLocalJoinPath()? If we don't do that, there won't be error building merge join plan and postgresRecheckForeignScan() would correctly route the EPQ checks to the local plan available as outer plan. Attached patch with that code removed. That might be fine for PG9.6, but I'd like to propose replacing GetExistingLocalJoinPath with CreateLocalJoinPath for PG10, because (1) GetExistingLocalJoinPath might choose an overkill, merge or hash join path for INNER/LEFT/SEMI/ANTI, not a nestloop join path, which might cause an overhead at EPQ rechecks, and (2) choosing a local join path randomly from the rel's pathlist wouldn't be easy to understand. I'll add this to the next CF. Best regards, Etsuro Fujita -- 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] Potential data loss of 2PC files
On Fri, Dec 23, 2016 at 3:02 AM, Andres Freund wrote: > Not quite IIRC: that doesn't deal with file size increase. All this would be > easier if hardlinks wouldn't exist IIUC. It's basically a question whether > dentry, inode or contents need to be synced. Yes, it sucks. I did more monitoring of the code... Looking at unlogged tables and empty() routines of access methods, isn't there a risk as well for unlogged tables? mdimmedsync() does not fsync() the parent directory either! It seems to me that we want to fsync() the parent folder in this code path especially and not just at checkpoint as this assumes that it does not care about shared buffers. We could do that at checkpoint as well, actually, by looping through all the tablespaces and fsync the database directories. Robert, as the former author of unlogged tables and Andres, as you have done a lot of work on durability, could you share your opinion on the matter? Of course opinions of others are welcome! -- Michael -- 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] Speedup twophase transactions
On Tue, Dec 27, 2016 at 12:59 PM, Stas Kelvich wrote: > Standard config with increased shared_buffers. I think the most significant > impact on the recovery speed here is on the client side, namely time between > prepare and commit. Right now I’m using pgbench script that issues commit > right after prepare. It’s also possible to put sleep between prepare and > commit > and increase number of connections to thousands. That will be probably the > worst case — majority of prepared tx will be moved to files. I think that it would be a good idea to actually test that in pure recovery time, aka no client, and just use a base backup and make it recover X prepared transactions that have created Y checkpoints after dropping cache (or restarting server). -- Michael -- 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] Speedup twophase transactions
> On 22 Dec 2016, at 05:35, Michael Paquier wrote: > > True. The more spread the checkpoints and 2PC files, the more risk to > require access to disk. Memory's cheap anyway. What was the system > memory? How many checkpoints did you trigger for how many 2PC files > created? Standard config with increased shared_buffers. I think the most significant impact on the recovery speed here is on the client side, namely time between prepare and commit. Right now I’m using pgbench script that issues commit right after prepare. It’s also possible to put sleep between prepare and commit and increase number of connections to thousands. That will be probably the worst case — majority of prepared tx will be moved to files. > Perhaps it would be a good idea to look for the 2PC files > from WAL records in a specific order. Did you try to use > dlist_push_head instead of dlist_push_tail? This may make a difference > on systems where WAL segments don't fit in system cache as the latest > files generated would be looked at first for 2PC data. Ouch! Good catch. I didn’t actually noticed that list populated in opposite order with respect to traversal. I’ll fix that. > On 27 Dec 2016, at 08:33, Michael Paquier wrote: > > Stas, have you tested as well tested the impact on recovery time when > WAL segments are very likely evicted from the OS cache? This could be > a plausible scenario if a standby instance is heavily used for > read-only transactions (say pgbench -S), and that the data quantity is > higher than the amount of RAM available. It would not be complicated > to test that: just drop_caches before beginning recovery. The maximum > amount of 2PC transactions that need to have access to the past WAL > segments is linearly related to the volume of WAL between two > checkpoints, so max_wal_size does not really matter. What matters is > the time it takes to recover the same amount of WAL. Increasing > max_wal_size would give more room to reduce the overall noise between > two measurements though. Okay, i’ll perform such testing. -- Stas Kelvich Postgres Professional: http://www.postgrespro.com The Russian Postgres Company
Re: [HACKERS] postgres_fdw bug in 9.6
On 2016/12/23 1:04, Robert Haas wrote: On Wed, Dec 21, 2016 at 11:04 AM, Ashutosh Bapat wrote: Some review comments 1. postgres_fdw doesn't push down semi and anti joins so you may want to discount these two too. + jointype == JOIN_SEMI || + jointype == JOIN_ANTI); But in the future, it might. I plan to work on adding those cases to postgres_fdw. We shouldn't randomly leave foot-guns lying around if there's an easy alternative. Some FDWs might have already supported pushing down semi/anti joins. So I think it's better to handle those joins as well. 3. Adding new members to JoinPathExtraData may save some work for postgres_fdw and other FDWs which would use CreateLocalJoinPath(), but it will add few bytes to the structure even when there is no "FULL foreign join which requires EPQ" involved in the query. That may not be so much of memory overhead since the structure is used locally to add_paths_to_joinrel(), but it might be something to think about. Instead, what if we call select_mergejoin_clauses() within CreateLocalJoinPath() to get that information? I think that's exactly backwards. The few bytes of storage don't matter, but extra CPU cycles might. I agree with Robert. Best regards, Etsuro Fujita -- 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] postgres_fdw bug in 9.6
On 2016/12/21 21:44, Etsuro Fujita wrote: On 2016/12/20 0:37, Tom Lane wrote: Etsuro Fujita writes: On 2016/12/17 1:13, Tom Lane wrote: So I think the rule could be "When first asked to produce a path for a given foreign joinrel, collect the cheapest paths for its left and right inputs, and make a nestloop path (or hashjoin path, if full join) from those, using the join quals needed for the current input relation pair. Seems reasonable. Use this as the fdw_outerpath for all foreign paths made for the joinrel." I'm not sure that would work well for foreign joins with sort orders. Consider a merge join, whose left input is a 2-way foreign join with a sort order that implements a full join and whose right input is a sorted local table scan. If the EPQ subplan for the foreign join wouldn't produce the right sort order, the merge join might break during EPQ rechecks (note that in this case the EPQ subplan for the foreign join might produce more than a single row during an EPQ recheck). How so? We only recheck one row at a time, therefore it can be claimed to have any sort order you care about. I'll have second thoughts about that. I noticed I was wrong and you are right. Sorry for the noise. Best regards, Etsuro Fujita -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Commit fest 2017-01 will begin soon!
Hi all, As mentioned a couple of weeks back, I am fine to work as commit fest manager for 2017-01. Here is the commit fest status at the moment I am writing this email: Needs review: 68. Waiting on Author: 17. Ready for Committer: 18. Committed: 27. Rejected: 2. Returned with Feedback: 6. Total: 138. So there are many of them, though there are not that many new entries as many patches have been moved from the last CF to this one. There are still a couple of days to register patches! So if you don't want your fancy feature to be forgotten, please add it in time to the CF app. Speaking of which, I am going to have a low bandwidth soon as that's a period of National Holidays in Japan for the new year, and I don't think I'll be able to mark the CF as in progress AoE time. So if somebody could do it for me that would be great :) Thanks, -- Michael -- 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] IF (NOT) EXISTS in psql-completion
Hello, At Mon, 26 Dec 2016 14:24:33 +0100, Pavel Stehule wrote in pavel.stehule> 2016-12-26 9:40 GMT+01:00 Kyotaro HORIGUCHI >: > > > > Thanks for reviewing but I ran out of time for this CF.. > > > > > > I'm going to move this to the next CF. > > > > I splitted the patch into small pieces. f3fd531 conflicted to > > this so rebased onto the current master HEAD. > > > > 0001 is psql_completion refactoring. > > 0002-0003 are patches prividing new infrastructures. > > 0004 is README for the infrastructures. > > 0005 is letter-case correction of SET/RESET/SHOW using 0002. > > 0006-0008 are improvements of recursive syntaxes using 0001 and 0004. > > 0009-0016 are simplifying (maybe) completion code per syntax. > > > > The last one (0017) is the IF(NOT)EXIST modifications. It > > suggests if(not)exists for syntaxes already gets object > > suggestion. So some kind of objects like operator, cast and so > > don't get an if.. suggestion. Likewise, I intentionally didn't > > modified siggestions for "TEXT SEARCH *". > > > > > lot of patches. I hope I look on these patches this week. Thank you for looking this and sorry for the many files. But I hople that they would be far easier to read. regards, -- Kyotaro Horiguchi NTT Open Source Software Center -- 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] [PATCH] Rename pg_switch_xlog to pg_switch_wal
On Thu, Dec 15, 2016 at 8:51 AM, Michael Paquier wrote: > On Thu, Dec 15, 2016 at 1:20 AM, Vladimir Rusinov wrote: >>> Personally, I think this is not important, but if you want to do it, I'd >>> follow the suggestion in the thread to rename all functions and leave >>> the old names as aliases or wrappers of some kind. >> >> I think I agree with Michael Paquier: "Better to do breakages in a single >> release rather than spreading them across releases.". >> There's going to be a lot of broken scripts following pg_xlog rename, so I >> think it makes sense to just drop functions as well. > > For consistency that makes sense in my view. But I won't be too noisy > as well if people think that we should keep aliases for compatibility. Actually, I am changing my mind on this bit, following Peter's opinion. Maintaining aliases of the past functions is a no-brainer, and it has no cost in terms of maintenance. That will save much pain to many monitoring scripts as well. -- Michael -- 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] BUG: pg_stat_statements query normalization issues with combined queries
Hello, At Tue, 27 Dec 2016 10:28:53 +0900 (Tokyo Standard Time), Kyotaro HORIGUCHI wrote in <20161227.102853.204155513.horiguchi.kyot...@lab.ntt.co.jp> > Putting the two above together, the following is my suggestion > for the parser part. > > - Make all parse nodes have the following two members at the > beginning. This unifies all parse node from the view of setting > their location. Commenting about this would be better. > > | NodeTag type; > | int location; > > - Remove struct ParseNode and setQueryLocation. stmtmulti sets > location in the same manner to other kind of nodes, just doing > '= @n'. base_yyparse() doesn't calculate statement length. > > - Make raw_parser caluclate statement length then store it into > each stmt scanning the yyextra.parsetree. An additional comment on parser(planner?) part. This patch make planner() copy the location and length from parse to result, but copying such stuffs is a job of standard_planner. Then the following is a comment on pg_stat_statements.c - pg_stat_statements.c:977 - isParseNode(node) node should be parsenode. - The name for the addional parameter query_loc is written as query_len in its prototype. - In pgss_store, "else if (strlen(query)) != query_len)" doesn't work as expected because of one-off error. query_len here is the length of the query *excluding* the last semicolon. - qtext_store doesn't rely on terminating nul character and the function already takes query length as a parameter. So, there's no need to copy the partial debug_query_string into palloc'ed memory. Just replacing the query with query_loc will be sufficient. Have a nice holidays. -- Kyotaro Horiguchi NTT Open Source Software Center -- 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] gettimeofday is at the end of its usefulness?
On Dec 26, 2016 10:35 PM, "Tom Lane" wrote: So it seems like the configure support we'd need is to detect whether clock_gettime is available (note on Linux there's also a library requirement, -lrt), and we would also need a way to provide a platform-specific choice of clockid; we at least need enough smarts to use CLOCK_MONOTONIC_RAW on macOS. This seems like something that really should be checked at runtime. It's very specific to the specific kernel you're running on, not the build environment, and it can hopefully be measured in only a second or even a fraction of a second. The only Pebblebrook would be if other things running on the system made the test results unpredictable so that you had a small chance of getting a very suboptimal choice and we ruling the dice each time you restarted...
Re: [HACKERS] Speedup twophase transactions
On Thu, Dec 22, 2016 at 7:35 AM, Michael Paquier wrote: > On Wed, Dec 21, 2016 at 10:37 PM, Stas Kelvich > wrote: >> ISTM your reasoning about filesystem cache applies here as well, but just >> without spending time on file creation. > > True. The more spread the checkpoints and 2PC files, the more risk to > require access to disk. Memory's cheap anyway. What was the system > memory? How many checkpoints did you trigger for how many 2PC files > created? Perhaps it would be a good idea to look for the 2PC files > from WAL records in a specific order. Did you try to use > dlist_push_head instead of dlist_push_tail? This may make a difference > on systems where WAL segments don't fit in system cache as the latest > files generated would be looked at first for 2PC data. Stas, have you tested as well tested the impact on recovery time when WAL segments are very likely evicted from the OS cache? This could be a plausible scenario if a standby instance is heavily used for read-only transactions (say pgbench -S), and that the data quantity is higher than the amount of RAM available. It would not be complicated to test that: just drop_caches before beginning recovery. The maximum amount of 2PC transactions that need to have access to the past WAL segments is linearly related to the volume of WAL between two checkpoints, so max_wal_size does not really matter. What matters is the time it takes to recover the same amount of WAL. Increasing max_wal_size would give more room to reduce the overall noise between two measurements though. -- Michael -- 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] BUG: pg_stat_statements query normalization issues with combined queries
Hello, At Mon, 26 Dec 2016 16:00:57 +0100 (CET), Fabien COELHO wrote in > > Hello Craig, > > > Please put me (ringerc) down as a reviewer. > > Done. > > Please do not loose time reviewing stupid version 1... skip to version > 2 directly:-) > > Also, note that the query-location part may be considered separately > from the pg_stat_statements part which uses it. In nonterminal stmtmulti, setQueryLocation is added and used to calcualte and store the length of every stmt's. This needs an extra storage in bse_yy_extra_type and introduces a bit complicated stuff. But I think raw_parser can do that without such extra storage and labor, then gram.y gets far simpler. The struct member to store the location and length is named 'qlocation', and 'qlength' in the added ParseNode. But many nodes already have 'location' of exactly the same purpopse. I don't see a necessity to differentiate them. Putting the two above together, the following is my suggestion for the parser part. - Make all parse nodes have the following two members at the beginning. This unifies all parse node from the view of setting their location. Commenting about this would be better. | NodeTag type; | int location; - Remove struct ParseNode and setQueryLocation. stmtmulti sets location in the same manner to other kind of nodes, just doing '= @n'. base_yyparse() doesn't calculate statement length. - Make raw_parser caluclate statement length then store it into each stmt scanning the yyextra.parsetree. What do you think about this? regards, -- Kyotaro Horiguchi NTT Open Source Software Center -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Support for pg_receivexlog --format=plain|tar
Hi all, Since 56c7d8d4, pg_basebackup supports tar format when streaming WAL records. This has been done by introducing a new transparent routine layer to control the method used to fetch WAL walmethods.c: plain or tar. pg_receivexlog does not make use of that yet, but I think that it could to allow retention of more WAL history within the same amount of disk space. OK, disk space is cheap but for some users things like that matters to define a duration retention policy. Especially when things are automated around Postgres. I really think that pg_receivexlog should be able to support an option like --format=plain|tar. "plain" is the default, and matches the current behavior. This option is of course designed to match pg_basebackup's one. So, here is in details what would happen if --format=tar is done: - When streaming begins, write changes to a tar stream, named segnum.tar.partial as long as the segment is not completed. - Once the segment completes, rename it to segnum.tar. - each individual segment has its own tarball. - if pg_receivexlog fails to receive changes in the middle of a segment, it begins streaming back at the beginning of a segment, considering that the current .partial segment is corrupted. So if server comes back online, empty the current .partial file and begin writing on it again. (I have found a bug on HEAD in this area actually). Magnus, you have mentioned me as well that you had a couple of ideas on the matter, feel free to jump in and let's mix our thoughts! There are a couple of things that I have been considering as well for pg_receivexlog. Though they are not directly stick to this thread, here they are as I don't forget about them: - Removal of oldest WAL segments on a partition. When writing WAL segments to a dedicated partition, we could have an option that automatically removes the oldest WAL segment if the partition is full. This triggers once a segment is completed. - Compression of fully-written segments. When a segment is finished being written, pg_receivexlog could compress them further with gz for example. With --format=t this leads to segnum.tar.gz being generated. The advantage of doing those two things in pg_receivexlog is monitoring. One process to handle them all, and there is no need of cron jobs to handle any cleanup or compression. Thanks, -- Michael -- 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] Questions regarding signal handler of postmaster
> But we keep signals blocked almost all the time in the postmaster, > so in reality no signal handler can interrupt anything except the > select() wait call. People complain about that coding technique > all the time, but no one has presented any reason to believe that > it's broken. Ok, there seems no better solution than always blocking signals. Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp -- 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] Questions regarding signal handler of postmaster
> I encountered that problem with postmaster and fixed it in 9.4.0 (it's not > back-patched to earlier releases because it's relatively complex). > > https://www.postgresql.org/message-id/20DAEA8949EC4E2289C6E8E58560DEC0@maumau > > > [Excerpt from 9.4 release note] > During crash recovery or immediate shutdown, send uncatchable termination > signals (SIGKILL) to child processes that do not shut down promptly (MauMau, > Álvaro Herrera) > This reduces the likelihood of leaving orphaned child processes behind after > postmaster shutdown, as well as ensuring that crash recovery can proceed if > some child processes have become “stuck”. Looks wild to me:-) I hope there exists better way to solve the problem... Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp -- 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] Questions regarding signal handler of postmaster
From: pgsql-hackers-ow...@postgresql.org > [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Tatsuo Ishii > In postmaster.c signal handler pmdie() calls ereport() and > errmsg_internal(), which could call palloc() then malloc() if necessary. > Because it is possible that pmdie() gets called while > malloc() gets called in postmaster, I think it is possible that a deadlock > situation could occur through an internal locking inside malloc(). I have > not observed the exact case in PostgreSQL but I see a suspected case in > Pgpool-II. In the stack trace #14, malloc() is called by Pgpool-II. It is > interrupted by a signal in #11, and the signal handler calls malloc() again, > and it is stuck at #0. I encountered that problem with postmaster and fixed it in 9.4.0 (it's not back-patched to earlier releases because it's relatively complex). https://www.postgresql.org/message-id/20DAEA8949EC4E2289C6E8E58560DEC0@maumau [Excerpt from 9.4 release note] During crash recovery or immediate shutdown, send uncatchable termination signals (SIGKILL) to child processes that do not shut down promptly (MauMau, Álvaro Herrera) This reduces the likelihood of leaving orphaned child processes behind after postmaster shutdown, as well as ensuring that crash recovery can proceed if some child processes have become “stuck”. Regards Takayuki Tsunakawa -- 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] Questions regarding signal handler of postmaster
Tatsuo Ishii writes: > In postmaster.c signal handler pmdie() calls ereport() and > errmsg_internal(), which could call palloc() then malloc() if > necessary. Because it is possible that pmdie() gets called while > malloc() gets called in postmaster, I think it is possible that a > deadlock situation could occur through an internal locking inside > malloc(). But we keep signals blocked almost all the time in the postmaster, so in reality no signal handler can interrupt anything except the select() wait call. People complain about that coding technique all the time, but no one has presented any reason to believe that it's broken. 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] Questions regarding signal handler of postmaster
In postmaster.c signal handler pmdie() calls ereport() and errmsg_internal(), which could call palloc() then malloc() if necessary. Because it is possible that pmdie() gets called while malloc() gets called in postmaster, I think it is possible that a deadlock situation could occur through an internal locking inside malloc(). I have not observed the exact case in PostgreSQL but I see a suspected case in Pgpool-II. In the stack trace #14, malloc() is called by Pgpool-II. It is interrupted by a signal in #11, and the signal handler calls malloc() again, and it is stuck at #0. So my question is, is my concern about PostgreSQL valid? If so, how can we fix it? #0 __lll_lock_wait_private () at ../nptl/sysdeps/unix/sysv/linux/x86_64/lowlevellock.S:95 #1 0x7f67fe20ccba in _L_lock_12808 () from /lib/x86_64-linux-gnu/libc.so.6 #2 0x7f67fe20a6b5 in __GI___libc_malloc (bytes=15) at malloc.c:2887 #3 0x7f67fe21072a in __GI___strdup (s=0x7f67fe305dd8 "/etc/localtime") at strdup.c:42 #4 0x7f67fe239f51 in tzset_internal (always=, explicit=explicit@entry=1) at tzset.c:444 #5 0x7f67fe23a913 in __tz_convert (timer=timer@entry=0x7ffce1c1b7f8, use_localtime=use_localtime@entry=1, tp=tp@entry=0x7f67fe54bde0 <_tmbuf>) at tzset.c:632 #6 0x7f67fe2387d1 in __GI_localtime (t=t@entry=0x7ffce1c1b7f8) at localtime.c:42 #7 0x0045627b in log_line_prefix (buf=buf@entry=0x7ffce1c1b8d0, line_prefix=, edata=) at ../../src/utils/error/elog.c:2059 #8 0x0045894d in send_message_to_server_log (edata=0x753320 ) at ../../src/utils/error/elog.c:2084 #9 EmitErrorReport () at ../../src/utils/error/elog.c:1129 #10 0x00456d8e in errfinish (dummy=) at ../../src/utils/error/elog.c:434 #11 0x00421f57 in die (sig=2) at protocol/child.c:925 #12 #13 _int_malloc (av=0x7f67fe546760 , bytes=4176) at malloc.c:3302 #14 0x7f67fe20a6c0 in __GI___libc_malloc (bytes=4176) at malloc.c:2891 Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp -- 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] gettimeofday is at the end of its usefulness?
Haribabu Kommi writes: > Attached a patch that replaces most of the getimeofday function calls, > except timeofday(user callable) and GetCurrentTimestamp functions. > Didn't add any configure checks in case if the clock_gettime function is > not available, the fallback logic to gettimeofday function call. Well, of course, configure is the hard part. I got interested in this area again pursuant to a question from Joel Jacobson, and looked around to see if things had changed any since 2014. One pleasant surprise is that Apple got around to implementing clock_gettime() as of the current macOS release (10.12 "Sierra"). That means that pretty much all interesting platforms now have clock_gettime(), which removes one possible objection to starting to use it. However, it seems like there is not a lot of commonality to the best "clockid" to use. In theory, according to the POSIX spec, CLOCK_MONOTONIC would be what we want to use for time interval measurement (EXPLAIN ANALYZE), since that would be impervious to possible clock setting changes. But some implementations might only optimize the more common CLOCK_REALTIME, and I found that there are a lot of platform-specific clock IDs that we might want to consider. On Linux (RHEL6, 2.4GHz x86_64), I find that gettimeofday(), clock_gettime(CLOCK_MONOTONIC), and clock_gettime(CLOCK_REALTIME) all take about 40ns. Of course gettimeofday() only has 1us resolution, but the other two have perhaps 10ns resolution (I get no duplicate readings in a tight loop). Other documented clockids include CLOCK_REALTIME_COARSE: about 10ns to read, but only 1ms resolution CLOCK_MONOTONIC_COARSE: about 12ns to read, but only 1ms resolution CLOCK_MONOTONIC_RAW: full resolution but very slow, ~145ns to read So CLOCK_MONOTONIC seems to be the thing to use here. It won't buy us anything speed-wise but the extra resolution will be nice. However, we need to do more research to see if this holds true on other popular distros. On macOS (10.12.2, 2.7GHz x86_64), clock_gettime(CLOCK_REALTIME) is actually a shade faster than gettimeofday: 40ns versus 46ns. But it's only giving 1us resolution, no better than gettimeofday. CLOCK_MONOTONIC is also 1us and it takes 75ns to read. But there's a CLOCK_MONOTONIC_RAW that takes 44ns to read and seems to offer full precision -- no duplicate readings in a tight loop. There's also CLOCK_MONOTONIC_RAW_APPROX which can be read in 23ns but the resolution is only around half an ms. I also tried FreeBSD 11.0 on another Mac (2.3GHz x86_64), and found that gettimeofday as well as basically all their clock_gettime variants run in 27 to 28 ns; and clock_gettime reliably delivers full precision, except for CLOCK_SECOND which is intentionally truncated to 1s precision. So there would be no need to work with anything but CLOCK_MONOTONIC here. However, it seems that these impressive results date back only to June 2012, cf https://github.com/freebsd/freebsd/commit/13a9f42818f6b89a72b3e40923be809b490400d8 and at least as of that commit, only x86 and x86_64 had the fast clock_gettime code. Older FreeBSD, or FreeBSD on another architecture, is likely to be a lot worse. But I lack an installation to try. I also tried OpenBSD 6.0 on that same Mac, and got pretty horrid results: gettimeofday, CLOCK_REALTIME, and CLOCK_MONOTONIC all take about 613ns to read. Ouch. And so does time(NULL); double ouch. Evidently there's no optimization on this platform and what we're seeing is the minimum cost for a kernel call. Still, we do get better precision from clock_gettime than gettimeofday, so we might as well switch. So it seems like the configure support we'd need is to detect whether clock_gettime is available (note on Linux there's also a library requirement, -lrt), and we would also need a way to provide a platform-specific choice of clockid; we at least need enough smarts to use CLOCK_MONOTONIC_RAW on macOS. 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] [PATCH] Generic type subscription
2016-12-26 18:49 GMT+03:00 Dmitry Dolgov <9erthali...@gmail.com>: >> On 5 December 2016 at 12:03, Haribabu Kommi >> wrote: > >> Moved to next CF with "needs review" status. > > Looks like we stuck here little bit. Does anyone else have any > suggestions/improvements, or this patch is in good enough shape? Would you rebase the patch, please? It seems it is necessary. It can't be applied now. -- Artur Zakirov Postgres Professional: http://www.postgrespro.com Russian Postgres Company -- 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] pg_stat_activity.waiting_start
Joel Jacobson writes: > Maybe a good tradeoff then would be to let "wait_start" represent the > very first time the txn started waiting? > ... > As long as the documentation is clear on "wait_start" meaning when the > first wait started in the txn, I think that's useful enough to improve > the situation, as one could then ask a query like "select all > processes that have possibly been waiting for at least 5 seconds", > which you cannot do today. Meh. You *can* do that now: query pg_stat_activity for waiting processes, wait a couple seconds, query again, intersect the results. I think really the only useful improvement on that would be to be able to tell that the process has been blocked continuously for X seconds, and what you're proposing here won't do that. In practice, there should never be waits on LWLocks (much less spinlocks) that exceed order-of-milliseconds; if there are, either we chose the wrong lock type or the system is pretty broken in general. So maybe it's sufficient if we provide a wait start time for heavyweight locks ... though that still seems kind of ugly. (Also, I don't recall the existing code factorization there, but getting the start time into pg_stat_activity without an extra gettimeofday call might be hard. As I said, there is one being done, but I'm not sure how accessible its result is.) I did a bit more research over the weekend into the cost of gettimeofday and clock_gettime, and have some new results that I'll post into that thread shortly. But the short answer is still "they're damn expensive on some platforms, and not really cheap anywhere". 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] Incautious handling of overlength identifiers
Alvaro Herrera writes: > I also admit it didn't occur to me to test the function(s) against > overlength input when developing it. I wouldn't object to adding code > to deal with overlength identifies, but I'm not really sure I would > choose truncation over reporting an error. But whatever it'd be, it'd > be at that level, not at the lower (hash function) level. Yeah, I'm now convinced that whatever we do about this, if we do anything, needs to be at a higher code level. It's not hashname()'s job to prevent use of overlength names. I'll go remove the Assert. 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] propose to pushdown qual into EXCEPT
I wrote: > tl;dr: I now think what the patch proposes to do is legit. There's a heck > of a lot of work to be done on the comments it's falsified, though. Hmm, wait a minute. I mentioned before that what nodeSetOp.c actually returns is N copies of the same representative tuple. That in itself doesn't break the proposed optimization, or at least so I argued --- but the real question is which representative tuple does it pick? The answer, as noted in the file header comment, is * Note that SetOp does no qual checking nor projection. The delivered * output tuples are just copies of the first-to-arrive tuple in each * input group. In HASHED mode, the first-to-arrive tuple must be from the lefthand input, which would mean that it's passed the pushed-down qual, so all is well. (If no LHS tuples exist in a given group, then EXCEPT won't output anything, so the fact that it could have collected a representative tuple from the RHS doesn't matter.) However, in SORTED mode, I don't see that there's anything particularly guaranteeing the order in which tuples arrive within a sort group. If the sort isn't stable, and I don't think all our sorting paths are, it would be possible to return an RHS tuple as the representative one. This breaks the proposed optimization because it would become possible to return a tuple that doesn't pass the pushed-down qual at all. There are at least two ways this could be dealt with. We could add the flag column as a low-order sort column so that it's still guaranteed that LHS tuples arrive before RHS ones within a group. (This'd complicate matters in generate_nonunion_path because now the sort keys would be different from the setop node's own grouping keys, but it's certainly possible.) Or we could fix it at runtime by complicating setop_retrieve_direct so that it replaces the representative tuple with the first LHS tuple when that arrives. Either way, though, more work is needed than just hacking the qual pushdown logic. 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] proposal: session server side variables
2016-12-26 19:13 GMT+01:00 Pavel Stehule : > > > 2016-12-26 18:20 GMT+01:00 Fabien COELHO : > >> >> >> >> And how would it interact with some "fully persistent/shared" variables? >> >> >> >> I have not any use case for this. I don't know about any similar feature >> in other database systems. Oracle uses dbms_pipe or dbms_alert for >> interprocess communication. >> >> I am thinking so it is possible to implement. If it is not ACID, then it >> can work as custom statistic counters. If it should be ACID? Then is better >> to use table. What I know, now is preferred share nothing design in >> parallel systems - so for me, it looks like little bit dangerous feature - >> and I see only one use case - custom statistics - where possible race >> condition is not hard issue. >> >> But I don't plan to implement it in first stage. There should be strong >> use case for implementing any complex feature in shared memory. But any >> implementation doesn't breaking to implement it in feature. >> > for custom statistic some extension based on bg worker can be better - sharing variables are risk of race conditions - and developers are people only. I have not a clean opinion about this - the implementation should not be hard, but I am not sure if this gun I would to give to users. Regards Pavel > >> Regards >> >> Pavel >> >> >> -- >> Fabien. > > >
Re: [HACKERS] Incautious handling of overlength identifiers
Tom Lane wrote: > I wrote: > > Another idea worth considering is to just make the low-level functions > > do truncation ... > > After further thought, there's a bigger-picture issue here, which > is whether the inputs to the SQL functions in question are intended to > be raw user input --- in which case, not only would truncation be > an appropriate service, but probably so would downcasing --- or whether > they are more likely to be coming from a catalog scan, in which case > you don't want any of that stuff. Nobody's going to be happy if we > start making them add quote_ident() around references to name columns. > I think the privilege-inquiry functions are almost certainly mostly > used in the latter style; there might be more room for debate about, > say, pg_get_serial_sequence. I expect that uses of pg_get_object_address() (one of the affected interfaces) would mostly be through an event trigger or a similar internal mechanism, that hopefully should appropriately quote names and not produce anything overlength. At least, pg_identify_object() (which is what I mostly had in mind) complies. I think removing the assert is a good enough solution, myself. I also admit it didn't occur to me to test the function(s) against overlength input when developing it. I wouldn't object to adding code to deal with overlength identifies, but I'm not really sure I would choose truncation over reporting an error. But whatever it'd be, it'd be at that level, not at the lower (hash function) level. -- Álvaro Herrerahttps://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- 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] proposal: session server side variables
2016-12-26 18:20 GMT+01:00 Fabien COELHO :
>
> Hello Pavel,
>
> I don't understand to "expensive" word.
>>
>
> Hmmm...
>
> How much often you create/drop these variables?
>>
>
> Hmmm... As for "session variables" à la MY/MS-SQL, ISTM that a variable is
> "created" each time it is asked for, and it disappears completely at the
> end of the session... So you can have some kind of minimal SQL scripting
> with variables executed server-side, without resorting to a PL. Also useful
> in interactive mode, although there the performance is less an issue.
>
> The deployment cycle is similar to functions. I don't
>> propose any feature, that can enforce bloating of system catalogue.
>>
>
> Hmmm
>
> The variable metadata will be persistent across sessions. I can imagine the
>> local (temporal) session variable where life cycle of metadata will be
>> related to session like our current temporary tables. But it is not in
>> this
>> proposal.
>>
>
> So it seems that I misunderstood a key detail in your proposal. Please
> accept my apology for my slow witedness. It is better to discuss in front
> of a white board...
>
sure, mainly with my language skills
>
> Now I understand that you want to create a kind of "persistant" session
> variable... that is at each new session the variable is instancianted in
> the session "automatically", whether the session will use it or not... Or
> is it instanciated on demand, i.e. when explicitely accessed?
>
I am starting with simple default solution - variable is initialized on
demand (when it is used first time in session). Attention: in my concept -
initialization and creation are different things.
In future initialization can be joined with login - we can have a autologin
function (trigger), or we can have a login initialized variables (and if
default expr is function call, then these functions can be used as
autologin functions).
>
> Could you confirm the interpretation? ISTM that "on demand" would be
> better.
>
Now, what I propose, and what is implemented in prototype is "on demand"
>
> As you gathered I was understanding that the "CREATE VARIABLE & GRANTS"
> were to be issued in each session for each variable used, inducing catalog
> changes each time, hence my quite heavy ranting...
>
> What I understand now is still a somehow strange object, but nevertheless
> the CREATE, DROP, GRANT, REVOKE at least are more justified because then
> object is somehow really persistent in the database, even if not with a
> value.
>
metadata are persistent like functions, sequences - the value is related to
session, only value.
> So I'll have to think about it...
>
> A few more questions:
>
> Bar the grantability, why wouldn't simple session variables work for this
> purpose? That is what is the added value of having them "declared"
> permanently, compared to created on demand, if the creation is really light
> weight?
>
the rights should be persistent, and should be attached to some persistent
object. Hypothetically, we can introduce new kind of objects, but it
disallow future usage of direct DML and SELECT statements.
>
> ISTM that in the Oracle package version, they are linked to PL/SQL, they
> are not part of SQL layer itself, so maybe they are only created when some
> PL/SQL from the package is invoked, and not created otherwise?
>
PL/SQL is different creature - it is living outside SQL catalogue - in
packages. I would not to reimplemented it for PL/pgSQL from following
reasons: we have schemas (that can be used as Oracle packages), we have a
extensions (that can be used partially as Oracle's packages), we have a mix
PL languages - more time I mixed PLpgSQL and PLPerlu. So mapping 1:1 from
Oracle is not good for Postgres.
>
> How would this feature interact with a kind of non persistent "simple"
> session variables that are found in MY/MS/Oracle SQL? One of my concern is
> that such a feature should not preclude other kind of session variables.
>
depends .. In my terminology your proposal is "untrusted temporary local
session variables" - it can share 50% of code - more if implementation will
be based on getter/setter function, less if it will be based on gram
implementation.
These variables should not be declared explicitly - it can be declared
implicitly by setting. They should not use any schema - bat can use
getter/setter functions
so you can write
select setvar('@var1', 10);
select getvar('@var1')
I little bit afraid of direct using the variables in query - inside special
functions we (and users) have control to choose volatility: direct using
can do some unexpected behave for users.
> And how would it interact with some "fully persistent/shared" variables?
>
I have not any use case for this. I don't know about any similar feature in
other database systems. Oracle uses dbms_pipe or dbms_alert for
interprocess communication.
I am thinking so it is possible to implement. If it is not ACID, then it
can work as custom statistic counters. If it should be
Re: [HACKERS] Incautious handling of overlength identifiers
I wrote: > Another idea worth considering is to just make the low-level functions > do truncation ... After further thought, there's a bigger-picture issue here, which is whether the inputs to the SQL functions in question are intended to be raw user input --- in which case, not only would truncation be an appropriate service, but probably so would downcasing --- or whether they are more likely to be coming from a catalog scan, in which case you don't want any of that stuff. Nobody's going to be happy if we start making them add quote_ident() around references to name columns. I think the privilege-inquiry functions are almost certainly mostly used in the latter style; there might be more room for debate about, say, pg_get_serial_sequence. Since the low-level functions need to support both use cases, asking them to handle truncation is wrong, just as much as it would be to ask them to do downcasing. If we take these SQL functions as being meant for use with inputs coming from catalogs, then they don't need to do truncation for user-friendliness purposes; it's perfectly fine to treat overlength inputs as "name not found" cases. So that says we could just remove that Assert and decide we're done. 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] proposal: session server side variables
Hello Pavel, I don't understand to "expensive" word. Hmmm... How much often you create/drop these variables? Hmmm... As for "session variables" à la MY/MS-SQL, ISTM that a variable is "created" each time it is asked for, and it disappears completely at the end of the session... So you can have some kind of minimal SQL scripting with variables executed server-side, without resorting to a PL. Also useful in interactive mode, although there the performance is less an issue. The deployment cycle is similar to functions. I don't propose any feature, that can enforce bloating of system catalogue. Hmmm The variable metadata will be persistent across sessions. I can imagine the local (temporal) session variable where life cycle of metadata will be related to session like our current temporary tables. But it is not in this proposal. So it seems that I misunderstood a key detail in your proposal. Please accept my apology for my slow witedness. It is better to discuss in front of a white board... Now I understand that you want to create a kind of "persistant" session variable... that is at each new session the variable is instancianted in the session "automatically", whether the session will use it or not... Or is it instanciated on demand, i.e. when explicitely accessed? Could you confirm the interpretation? ISTM that "on demand" would be better. As you gathered I was understanding that the "CREATE VARIABLE & GRANTS" were to be issued in each session for each variable used, inducing catalog changes each time, hence my quite heavy ranting... What I understand now is still a somehow strange object, but nevertheless the CREATE, DROP, GRANT, REVOKE at least are more justified because then object is somehow really persistent in the database, even if not with a value. So I'll have to think about it... A few more questions: Bar the grantability, why wouldn't simple session variables work for this purpose? That is what is the added value of having them "declared" permanently, compared to created on demand, if the creation is really light weight? ISTM that in the Oracle package version, they are linked to PL/SQL, they are not part of SQL layer itself, so maybe they are only created when some PL/SQL from the package is invoked, and not created otherwise? How would this feature interact with a kind of non persistent "simple" session variables that are found in MY/MS/Oracle SQL? One of my concern is that such a feature should not preclude other kind of session variables. And how would it interact with some "fully persistent/shared" variables? -- Fabien. -- 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] Incautious handling of overlength identifiers
Michael Paquier writes: > On Sat, Dec 24, 2016 at 7:44 AM, Joe Conway wrote: >> On 12/23/2016 12:44 PM, Tom Lane wrote: >>> An alternative worth considering, especially for the back branches, >>> is simply to remove the Assert in hashname(). That would give us >>> the behavior that non-developers see anyway, which is that these >>> functions always fail to match overlength names, whether or not >>> the names would have matched after truncation. Trying to apply >>> truncation more consistently could be left as an improvement >>> project for later. >> That sounds reasonable to me. > +1 for just removing the assertion on back-branches. On HEAD, it seems > right to me to keep the assertion. However it is not possible to just > switch those routines from text to name as a table could be defined > with its schema name. So at minimum this would require adjusting > textToQualifiedNameList() & similar routines in charge of putting in > shape the name lists. textToQualifiedNameList() already does truncate, and I suspect everyplace that deals in qualified names does as well. The problem places are those where someone just took a text parameter, did text_to_cstring on it, and started treating the result as a Name (e.g. by passing it to a catalog lookup function). Since we've intentionally allowed C strings to be used as Names in most places, it's not even immediately obvious that this is wrong. I'm currently inclined to think that removing the assertion from hashname() is right even for HEAD, because it's not very relevant to the operation of that routine (you'll get back some hash value even for overlength strings), and because it's a pretty useless way of enforcing truncation. It basically will only catch you if you try to do a syscache lookup to resolve a name --- if you do a catalog heap or index scan, those paths contain no such gotcha. And I'm disinclined to introduce one. The real problem with trying to enforce this through length assertions in low-level routines is that they'll only reveal a bug if you actually happen to test the appropriate calling code path with an overlength name. We've obviously failed to do that in the past and I have little faith that we'd do it in the future. So that's why I was thinking about whether we could do this through some datatype-based approach, whereby we could hope to catch incorrect coding reliably through compiler checks. But given our history of allowing C strings as names, I'm afraid that any such change would be enormously invasive and not worth the trouble. Another idea worth considering is to just make the low-level functions do truncation, ie the fix in hashname would look more like - Assert(keylen < NAMEDATALEN); + if (keylen >= NAMEDATALEN) + keylen = pg_mbcliplen(key, keylen, NAMEDATALEN - 1); and we'd need something similar in the name comparison functions. But that would be slightly annoying from a performance standpoint. Not so much the extra pg_mbcliplen calls, because we could assume those wouldn't happen in any performance-interesting cases; but there are no strlen calls in the name comparison functions right now, so we'd have to add some, and those would get hit every time through. 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] proposal: session server side variables
2016-12-26 17:33 GMT+01:00 Fabien COELHO : > > please, can send link? >> > > My badly interpreted PL/SQL example was on the same page you point to > below: > > so some better documentation >> https://docs.oracle.com/cd/E11882_01/appdev.112/e25519/packa >> ges.htm#LNPLS99926 >> > > There is a private 'number_hired' which given its name I thought was > counting the number of employee, but it was just counting the number of > "hire_employee" calls in the current session... Not very interesting. > > I am sure, so package variables are not shared between sessions/backends >> > > Indeed, I misinterpreted the Oracle documentation example. > > > [ grantable function example to access a private session variable... ] >>> >> >> I am sorry, it is not secure. Theoretically it can work if you have >> granted order of function calls, but if not? >> > > I'm not sure I understand. > > If you do not grant/revoke permissions as you want on the functions, then > it can be invoked by anybody. > > My point is that it is *possible* to tune permissions so as to control > exactly who may access a private session variable. > > That is exactly the same with a grantable session variable if you do not > have done the necessary grant/revoke, there is no difference? > If you use pattern DECLARE IF NOT EXISTS, you cannot be sure so some other did it. It is working only if you create variables in session as first. Only if object is fixed in schema, then object is trustworthy - because you have to have rights to modify schema. In my proposal only trustworthy user can create the variable in some schema. Not trustworthy user can use public schema, or we can support temporary objects (similar to your proposal) in hypothetical schema "private". I have strong tools in Postgres for enforcing security, and I would to use it. Regards Pavel > > -- > Fabien. >
Re: [HACKERS] proposal: session server side variables
please, can send link? My badly interpreted PL/SQL example was on the same page you point to below: so some better documentation https://docs.oracle.com/cd/E11882_01/appdev.112/e25519/packages.htm#LNPLS99926 There is a private 'number_hired' which given its name I thought was counting the number of employee, but it was just counting the number of "hire_employee" calls in the current session... Not very interesting. I am sure, so package variables are not shared between sessions/backends Indeed, I misinterpreted the Oracle documentation example. [ grantable function example to access a private session variable... ] I am sorry, it is not secure. Theoretically it can work if you have granted order of function calls, but if not? I'm not sure I understand. If you do not grant/revoke permissions as you want on the functions, then it can be invoked by anybody. My point is that it is *possible* to tune permissions so as to control exactly who may access a private session variable. That is exactly the same with a grantable session variable if you do not have done the necessary grant/revoke, there is no difference? -- Fabien. -- 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] proposal: session server side variables
2016-12-26 17:15 GMT+01:00 Fabien COELHO :
>
> Hello Pavel,
>
> SET ROLE Admin;
>>> DECLARE @secure_variable INTEGER RESTRICT; -- only accessible to Admin
>>>
>>
> Why introduce another security system?
>>
>
> That is a good question.
>
> I would prefer to avoid it and just have simple session variables... but
> this is not what you want, so I'm trying to find a compromise which both
> gives you the feature you are seeking and would keep session variables as
> inexpensive, i.e. without catalog costs.
>
> A simplistic PUBLIC/PRIVATE permissions on simple session variable can be
> done with nothing (no additional data structures): (store: hash_put( id if private or nothing> || '@varname', value); consult: if exists( id> || '@varname') then return it else if exists('@varname') then return it
> else "error variable does not exist").
>
> Now if you can offer an inexpensive GRANT/REVOKE on simple session
> variables, i.e. without catalog changes, then I think I would agree to it,
> even if I would not like it much.
>
> The reason I "do not like much" is subjective. I associate SQL permission
> commands (GRANT, REVOKE...) to real SQL "objects" (i.e. persistent,
> transactional, secured things subject to CREATE ALTER DROP...). However
> light-weight session variables are not really like that.
>
> Also if you can have inexpensive GRANT/REVOKE then probably I would also
> have to accept "CREATE SESSION VARIABLE @foo", because it would be
> consistent to have it with GRANT/REVOKE. I would "not like it much" either
> to have CREATE for an non persistant object, but that is life...
>
> However I understood that for permissions you do need "pg_class", which
> means catalog changes on session variable creation, which means expensive
> for simple session variables, so not desirable.
>
I don't understand to "expensive" word. How much often you create/drop
these variables? The deployment cycle is similar to functions. I don't
propose any feature, that can enforce bloating of system catalogue.
The variable metadata will be persistent across sessions. I can imagine the
local (temporal) session variable where life cycle of metadata will be
related to session like our current temporary tables. But it is not in this
proposal.
Regards
Pavel
> --
> Fabien.
>
Re: [HACKERS] proposal: session server side variables
Hello Pavel,
SET ROLE Admin;
DECLARE @secure_variable INTEGER RESTRICT; -- only accessible to Admin
Why introduce another security system?
That is a good question.
I would prefer to avoid it and just have simple session variables... but
this is not what you want, so I'm trying to find a compromise which both
gives you the feature you are seeking and would keep session variables as
inexpensive, i.e. without catalog costs.
A simplistic PUBLIC/PRIVATE permissions on simple session variable can be
done with nothing (no additional data structures): (store: hash_put(id if private or nothing> || '@varname', value); consult: if exists(id> || '@varname') then return it else if exists('@varname') then return
it else "error variable does not exist").
Now if you can offer an inexpensive GRANT/REVOKE on simple session
variables, i.e. without catalog changes, then I think I would agree to it,
even if I would not like it much.
The reason I "do not like much" is subjective. I associate SQL permission
commands (GRANT, REVOKE...) to real SQL "objects" (i.e. persistent,
transactional, secured things subject to CREATE ALTER DROP...). However
light-weight session variables are not really like that.
Also if you can have inexpensive GRANT/REVOKE then probably I would also
have to accept "CREATE SESSION VARIABLE @foo", because it would be
consistent to have it with GRANT/REVOKE. I would "not like it much" either
to have CREATE for an non persistant object, but that is life...
However I understood that for permissions you do need "pg_class", which
means catalog changes on session variable creation, which means expensive
for simple session variables, so not desirable.
--
Fabien.
--
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] Patch: Write Amplification Reduction Method (WARM)
Alvaro Herrera wrote: > Jaime Casanova wrote: > > > * The isolation test for partial_index fails (attached the regression.diffs) > > Hmm, I had a very similar (if not identical) failure with indirect > indexes; in my case it was a bug in RelationGetIndexAttrBitmap() -- I > was missing to have HOT considerate the columns in index predicate, that > is, the second pull_varattnos() call. Sorry, I meant: Hmm, I had a very similar (if not identical) failure with indirect indexes; in my case it was a bug in RelationGetIndexAttrBitmap() -- I was missing to have HOT [take into account] the columns in index predicate, that is, the second pull_varattnos() call. -- Álvaro Herrerahttps://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- 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] Patch: Write Amplification Reduction Method (WARM)
Jaime Casanova wrote: > * The isolation test for partial_index fails (attached the regression.diffs) Hmm, I had a very similar (if not identical) failure with indirect indexes; in my case it was a bug in RelationGetIndexAttrBitmap() -- I was missing to have HOT considerate the columns in index predicate, that is, the second pull_varattnos() call. -- Álvaro Herrerahttps://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- 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] [PATCH] Generic type subscription
> On 5 December 2016 at 12:03, Haribabu Kommi wrote: > > Moved to next CF with "needs review" status. Looks like we stuck here little bit. Does anyone else have any suggestions/improvements, or this patch is in good enough shape?
Re: [HACKERS] proposal: session server side variables
2016-12-26 15:53 GMT+01:00 Fabien COELHO : > > Hello Pavel, > > AFAICS they are shared between backends, [...] They constitute a >>> consistent design. >>> >> >> no >> http://stackoverflow.com/questions/2383061/scope-of-oracle- >> package-level-variables >> > > If stackoverflow says so, too bad for me:-) Now I do not understand the > point of the example I read on Oracle documentation: why having an employee > count accessed by some functions if it is reset on each new session? > please, can send link? so some better documentation https://docs.oracle.com/cd/E11882_01/appdev.112/e25519/packages.htm#LNPLS99926 I am sure, so package variables are not shared between sessions/backends - bacause Oracle uses different mechanism for interprocess communication - wrote it in Orafce "When a session references a package item, Oracle Database instantiates the package for that session. Every session that references a package has its own instantiation of that package." > > So I do retract "it constitute a consistent design". It looks more like a > PL/SQL confined hack. > > Note that Oracle also seems to have session variables with set with DEFINE > and referenced with &variable. > > [...] That could look like: >>> >>> SET ROLE Admin; >>> DECLARE @secure_variable INTEGER RESTRICT; -- only accessible to Admin >>> SET @secure_variable = 3; >>> >>> SET ROLE BasicUser; >>> SELECT @secure_variable; -- say NULL or error does not exist... >>> >>> what will be if BasicUser does DECLARE @secure_variable >> > > Then there would be a distinct global @secure_variable unrelated to the > previous one, that would be hidden from Admin who would see its own private > @secure_variable. Maybe "restrict" is not the right word, though, let us > use "private". > >SET ROLE User1; >-- use @var: does not exist in scope error >DECLARE @var INTEGER PRIVATE; >SET @var = 1; >-- use @var: get 1 > >SET ROLE User2; >-- use @var: does not exist in scope error >DECLARE @var INTEGER PUBLIC; >SET @var = 2; >-- use @var; get 2 > >SET ROLE User1; >-- use @var: get 1 (private version) > >SET ROLE User3; >-- use @var: get 2 (public version created by User2). > > > There are not any granularity of rights - you cannot to grant access ... >> > > Indeed, at least directly. With the above version you can just control > whether everybody or only the owner has access. > > However with some minimal more effort the owner of a private session > variable could provide a grantable function for accessing this variable: > the benefit would be that the function is permanent, i.e. would not need to > be granted each time the variable is used, it could be done once and for > all. > > CREATE FUNCTION setSecret(INT) SECURITY DEFINER ... AS $$ > DECLARE IF NOT EXISTS @secret TEXT PRIVATE; > SET @secret = $1; > $$ LANGUAGE SQL; > > CREATE FUNCTION useSecret(TEXT) SECURITY DEFINER TEXT AS $$ > -- would fail if @secret has not been set yet... > SELECT sha256sum(@secret || ':' || $1); > $$ LANGUAGE SQL; > > CREATE FUNCTION getSecret() RETURNS TEXT SECURITY DEFINER AS $$ > DECLARE IF NOT EXISTS @secret TEXT PRIVATE; > SELECT @secret; > $$ LANGUAGE SQL; > > -- then > REVOKE/GRANT ... ON FUNCTION set/use/getSecret(...); > > I am sorry, I don't see benefit in your proposal. >> > > The benefit I see is to have MS/MY-SQL/Oracle like light-weight > (inexpensive, untransactional) session variables and still a minimal access > control which might be enough for significant use cases. > > If more is really needed, consider the function hack, or maybe some > one-row table with all the power of grant. Ok, the table solution is more > heavy weight, but then this is also for a special requirement, and it would > work as well for persistence. > > Probably there will be only one agreement, so there are not agreement >> between us :( >> > > It seems so. I do believe that I am trying to propose a solution which > take into account your use case as I understand it (you did not confirm nor > infirm) which is to store securely but not safely some kind of temporary > data between different function calls with SECURITY DEFINER within the same > session. > > I'm trying to avoid "special-case" medium-weight (i.e. pg_class-based) > session variables with permissions, which could preclude MY/MS-SQL/Oracle > like light-weight session variables which are I think interesting in their > own right. > I am sorry, it is not secure. Theoretically it can work if you have granted order of function calls, but if not? regards Pavel > > -- > Fabien. >
Re: [HACKERS] BUG: pg_stat_statements query normalization issues with combined queries
Hello Craig, Please put me (ringerc) down as a reviewer. Done. Please do not loose time reviewing stupid version 1... skip to version 2 directly:-) Also, note that the query-location part may be considered separately from the pg_stat_statements part which uses it. -- Fabien. -- 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] proposal: session server side variables
Hello Pavel, AFAICS they are shared between backends, [...] They constitute a consistent design. no http://stackoverflow.com/questions/2383061/scope-of-oracle-package-level-variables If stackoverflow says so, too bad for me:-) Now I do not understand the point of the example I read on Oracle documentation: why having an employee count accessed by some functions if it is reset on each new session? So I do retract "it constitute a consistent design". It looks more like a PL/SQL confined hack. Note that Oracle also seems to have session variables with set with DEFINE and referenced with &variable. [...] That could look like: SET ROLE Admin; DECLARE @secure_variable INTEGER RESTRICT; -- only accessible to Admin SET @secure_variable = 3; SET ROLE BasicUser; SELECT @secure_variable; -- say NULL or error does not exist... what will be if BasicUser does DECLARE @secure_variable Then there would be a distinct global @secure_variable unrelated to the previous one, that would be hidden from Admin who would see its own private @secure_variable. Maybe "restrict" is not the right word, though, let us use "private". SET ROLE User1; -- use @var: does not exist in scope error DECLARE @var INTEGER PRIVATE; SET @var = 1; -- use @var: get 1 SET ROLE User2; -- use @var: does not exist in scope error DECLARE @var INTEGER PUBLIC; SET @var = 2; -- use @var; get 2 SET ROLE User1; -- use @var: get 1 (private version) SET ROLE User3; -- use @var: get 2 (public version created by User2). There are not any granularity of rights - you cannot to grant access ... Indeed, at least directly. With the above version you can just control whether everybody or only the owner has access. However with some minimal more effort the owner of a private session variable could provide a grantable function for accessing this variable: the benefit would be that the function is permanent, i.e. would not need to be granted each time the variable is used, it could be done once and for all. CREATE FUNCTION setSecret(INT) SECURITY DEFINER ... AS $$ DECLARE IF NOT EXISTS @secret TEXT PRIVATE; SET @secret = $1; $$ LANGUAGE SQL; CREATE FUNCTION useSecret(TEXT) SECURITY DEFINER TEXT AS $$ -- would fail if @secret has not been set yet... SELECT sha256sum(@secret || ':' || $1); $$ LANGUAGE SQL; CREATE FUNCTION getSecret() RETURNS TEXT SECURITY DEFINER AS $$ DECLARE IF NOT EXISTS @secret TEXT PRIVATE; SELECT @secret; $$ LANGUAGE SQL; -- then REVOKE/GRANT ... ON FUNCTION set/use/getSecret(...); I am sorry, I don't see benefit in your proposal. The benefit I see is to have MS/MY-SQL/Oracle like light-weight (inexpensive, untransactional) session variables and still a minimal access control which might be enough for significant use cases. If more is really needed, consider the function hack, or maybe some one-row table with all the power of grant. Ok, the table solution is more heavy weight, but then this is also for a special requirement, and it would work as well for persistence. Probably there will be only one agreement, so there are not agreement between us :( It seems so. I do believe that I am trying to propose a solution which take into account your use case as I understand it (you did not confirm nor infirm) which is to store securely but not safely some kind of temporary data between different function calls with SECURITY DEFINER within the same session. I'm trying to avoid "special-case" medium-weight (i.e. pg_class-based) session variables with permissions, which could preclude MY/MS-SQL/Oracle like light-weight session variables which are I think interesting in their own right. -- Fabien. -- 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] BUG: pg_stat_statements query normalization issues with combined queries
On 23 Dec. 2016 17:53, "Fabien COELHO" wrote: Yes. I'll try to put together a patch and submit it to the next CF. > Here it is. I'll add this to the next CF. Great. Please put me (ringerc) down as a reviewer. I'll get to this as soon as holidays settle down. It'd be very useful for some things I'm working on too, and is much better then my early draft of similar functionality.
Re: [HACKERS] BUG: pg_stat_statements query normalization issues with combined queries
On 21 Dec. 2016 11:44, "Robert Haas" wrote: On Tue, Dec 20, 2016 at 6:18 AM, Fabien COELHO wrote: > Would this approach be acceptable, or is modifying Nodes a no-go area? > > If it is acceptable, I can probably put together a patch and submit it. > > If not, I suggest to update the documentation to tell that > pg_stat_statements does not work properly with combined queries. I think you've found a bug, but I'm a little doubtful about your proposed fix. However, I haven't studied the code, so I don't know what other approach might be better. FWIW this issue with multi-statements also causes issues with ProcessUtility_hook. It gets the char* querytext of the whole multistatement. Then gets invoked once for each utility command within. It has no information about which statement text the current invocation corresponds to. Having a.pointer into the query text for the start and end would be good there too. Not as good as doing away with multis entirely as a bad hack but that's not practical for BC and protocol reasons. BTW we should be sure the somewhat wacky semantics of multi-statements with embedded commits are documented. I'll check tomorrow.
Re: [HACKERS] IF (NOT) EXISTS in psql-completion
2016-12-26 9:40 GMT+01:00 Kyotaro HORIGUCHI : > > Thanks for reviewing but I ran out of time for this CF.. > > > > I'm going to move this to the next CF. > > I splitted the patch into small pieces. f3fd531 conflicted to > this so rebased onto the current master HEAD. > > 0001 is psql_completion refactoring. > 0002-0003 are patches prividing new infrastructures. > 0004 is README for the infrastructures. > 0005 is letter-case correction of SET/RESET/SHOW using 0002. > 0006-0008 are improvements of recursive syntaxes using 0001 and 0004. > 0009-0016 are simplifying (maybe) completion code per syntax. > > The last one (0017) is the IF(NOT)EXIST modifications. It > suggests if(not)exists for syntaxes already gets object > suggestion. So some kind of objects like operator, cast and so > don't get an if.. suggestion. Likewise, I intentionally didn't > modified siggestions for "TEXT SEARCH *". > > lot of patches. I hope I look on these patches this week. Regards Pavel > regards, > > -- > Kyotaro Horiguchi > NTT Open Source Software Center >
Re: [HACKERS] proposal: session server side variables
> >> A possible compromise I have proposed is to have some declared access >> restrictions on simple session variables, so that say only the owner can >> access it, but they should stay and look like light-weight session >> variables nevertheless. That could look like: >> >> SET ROLE Admin; >> DECLARE @secure_variable INTEGER RESTRICT; -- only accessible to Admin >> > Why introduce any another security system? Regards Pavel
Re: [HACKERS] proposal: session server side variables
2016-12-26 13:08 GMT+01:00 Fabien COELHO : > > > Hello Pavel, > > you are talk about light session variables like MSSQL or MySQL (with same >> syntax), I am talking about secure session variables like Oracle package >> variables (with similar access syntax). >> > > Hmmm. I do not know this Oracle stuff... After looking at the online > documentation, my understanding of "Oracle package variables" refers to > full fledged database objects, in particular they are not session limited. > The example I found is about a variable holding the total number of > employees, with functions hire & fire (well, they call it remove_emp) to > update them when inserting or deleting an employee. > > AFAICS they are shared between backends, subjects to transactions and > permissions, constraints and so on. So they look more like the first > category I outlined, and probably they cost as any persistent database > object, which make sense. They constitute a consistent design. > no http://stackoverflow.com/questions/2383061/scope-of-oracle-package-level-variables > > > This is * not * what you are proposing. > > > [...] I have two important reasons why I insist on pg_class base. >> >> 1. security .. it is really fundamental part >> 2. possibility to static check by plpgsql_check - without entry in >> pg_class >> (or other catalogue table) I have not any valid information about type, >> existence of any variable. >> > > Hmmm. I'm not quite convinced that putting session variables in pg_class > is a good idea, because it adds significant costs for the use case of > "standard" simple session variables, which is quite more probable than > session-but-with-permissions variables. > > As far as security is concerned, ISTM that a limited but still useful > access control can be implemented for a key-value store with simple session > variables, see below. > > As far as typing is concerned, ISTM that it can be done as well for > session variables by going through text and using casts when setting and > getting values, or through some other simple ad-hoc checking. > > Although I am not supporter (due possible issues with plpgsql_checks) of >> MySQL or MSSQL style variables I am not strongly against this >> implementation with same syntax. But it is different feature, with >> different benefits and costs. >> > > I didn't proposed the packages (and package variables) due issues in >> multilingual PostgreSQL environment and because it is redundant to >> PostgreSQL schemas. >> > > Instead I proposed >>secure global session variables<< (global like global >> temporary tables). >> > > That's where I'm leaving you and start disagreeing, because it is not > consistent: you are proposing session variables that do not look like > session variable and are somehow costly. > > I could agree with real "secure global variables" as in Oracle packages, a > consistent kind of database object which stores a persistent value safely > and securely. That would cost, but that is life in a database, you have > great things for a price. Probably that could be implemented as a row in > some special table, or as a one-row table, or whatever. > > I could also agree with à la MS or MY-SQL session variables that look like > session variables, with limited ambition, light-weight and inexpensive. > > I disagree with having a half-backed stuff, where something looks like a > database object (i.e. CREATE/ALTER/DROP/GRANT/REVOKE) but is really a > session object with strange properties. I also disagree to the pg_class > approach as it creates in effect an expensive session object while a simple > session object would cost much less and would be much more useful. > > To summarize, I still think that your design is not consistent, even if it > makes sense for some degree wrt the implementation. > > A possible compromise I have proposed is to have some declared access > restrictions on simple session variables, so that say only the owner can > access it, but they should stay and look like light-weight session > variables nevertheless. That could look like: > > SET ROLE Admin; > DECLARE @secure_variable INTEGER RESTRICT; -- only accessible to Admin > SET @secure_variable = 3; > > SET ROLE BasicUser; > SELECT @secure_variable; -- say NULL or error does not exist... > > what will be if BasicUser does DECLARE @secure_variable There are not any granularity of rights - you cannot to grant access ... > SET ROLE Admin; > SELECT @secure_variable; 3 > I am sorry, I don't see benefit in your proposal. Probably there will be only one agreement, so there are not agreement between us :( Regards Pavel > ... > > Currently light session variables can be implemented as not big extension. >> > > Sure. I would review that as well. > > -- > Fabien.
Re: [HACKERS] proposal: session server side variables
Hello Pavel, you are talk about light session variables like MSSQL or MySQL (with same syntax), I am talking about secure session variables like Oracle package variables (with similar access syntax). Hmmm. I do not know this Oracle stuff... After looking at the online documentation, my understanding of "Oracle package variables" refers to full fledged database objects, in particular they are not session limited. The example I found is about a variable holding the total number of employees, with functions hire & fire (well, they call it remove_emp) to update them when inserting or deleting an employee. AFAICS they are shared between backends, subjects to transactions and permissions, constraints and so on. So they look more like the first category I outlined, and probably they cost as any persistent database object, which make sense. They constitute a consistent design. This is * not * what you are proposing. [...] I have two important reasons why I insist on pg_class base. 1. security .. it is really fundamental part 2. possibility to static check by plpgsql_check - without entry in pg_class (or other catalogue table) I have not any valid information about type, existence of any variable. Hmmm. I'm not quite convinced that putting session variables in pg_class is a good idea, because it adds significant costs for the use case of "standard" simple session variables, which is quite more probable than session-but-with-permissions variables. As far as security is concerned, ISTM that a limited but still useful access control can be implemented for a key-value store with simple session variables, see below. As far as typing is concerned, ISTM that it can be done as well for session variables by going through text and using casts when setting and getting values, or through some other simple ad-hoc checking. Although I am not supporter (due possible issues with plpgsql_checks) of MySQL or MSSQL style variables I am not strongly against this implementation with same syntax. But it is different feature, with different benefits and costs. I didn't proposed the packages (and package variables) due issues in multilingual PostgreSQL environment and because it is redundant to PostgreSQL schemas. Instead I proposed >>secure global session variables<< (global like global temporary tables). That's where I'm leaving you and start disagreeing, because it is not consistent: you are proposing session variables that do not look like session variable and are somehow costly. I could agree with real "secure global variables" as in Oracle packages, a consistent kind of database object which stores a persistent value safely and securely. That would cost, but that is life in a database, you have great things for a price. Probably that could be implemented as a row in some special table, or as a one-row table, or whatever. I could also agree with à la MS or MY-SQL session variables that look like session variables, with limited ambition, light-weight and inexpensive. I disagree with having a half-backed stuff, where something looks like a database object (i.e. CREATE/ALTER/DROP/GRANT/REVOKE) but is really a session object with strange properties. I also disagree to the pg_class approach as it creates in effect an expensive session object while a simple session object would cost much less and would be much more useful. To summarize, I still think that your design is not consistent, even if it makes sense for some degree wrt the implementation. A possible compromise I have proposed is to have some declared access restrictions on simple session variables, so that say only the owner can access it, but they should stay and look like light-weight session variables nevertheless. That could look like: SET ROLE Admin; DECLARE @secure_variable INTEGER RESTRICT; -- only accessible to Admin SET @secure_variable = 3; SET ROLE BasicUser; SELECT @secure_variable; -- say NULL or error does not exist... SET ROLE Admin; SELECT @secure_variable; 3 ... Currently light session variables can be implemented as not big extension. Sure. I would review that as well. -- Fabien. -- 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] Declarative partitioning - another take
Sorry about the delay in replying. On 2016/12/23 8:08, Robert Haas wrote: > On Thu, Dec 22, 2016 at 3:35 AM, Amit Langote > wrote: >> While working on that, I discovered yet-another-bug having to do with the >> tuple descriptor that's used as we route a tuple down a partition tree. If >> attnums of given key attribute(s) are different on different levels, it >> would be incorrect to use the original slot's (one passed by ExecInsert()) >> tuple descriptor to inspect the original slot's heap tuple, as we go down >> the tree. It might cause spurious "partition not found" at some level due >> to looking at incorrect field in the input tuple because of using the >> wrong tuple descriptor (root table's attnums not always same as other >> partitioned tables in the tree). Patch 0001 fixes that including a test. > > I committed this, but I'm a bit uncomfortable with it: should the > TupleTableSlot be part of the ModifyTableState rather than the EState? Done that way in 0001 of the attached patches. So, instead of making the standalone partition_tuple_slot a field of EState (with the actual TupleTableSlot in its tupleTable), it is now allocated within ModifyTableState and CopyState, and released when ModifyTable node or CopyFrom finishes, respectively. >> It also addresses the problem I mentioned previously that once >> tuple-routing is done, we failed to switch to a slot with the leaf >> partition's tupdesc (IOW, continued to use the original slot with root >> table's tupdesc causing spurious failures due to differences in attums >> between the leaf partition and the root table). >> >> Further patches 0002, 0003 and 0004 fix bugs that I sent one-big-patch for >> in my previous message. Each patch has a test for the bug it's meant to fix. > > Regarding 0002, I think that this is kind of a strange fix. Wouldn't > it be better to get hold of the original tuple instead of reversing > the conversion? And what of the idea of avoiding the conversion in > the (probably very common) case where we can? To get hold of the original tuple, how about adding an argument orig_slot to ExecConstraints()? I've implemented that approach in the new 0002. Regarding the possibility of avoiding the conversion in very common cases, I think that could be done considering the following: If the mapping from the attribute numbers of the parent table to that of a child table is an identity map, we don't need to convert tuples. Currently however, convert_tuples_by_name() also requires tdtypeid of the input and output TupleDescs to be equal. The reason cited for that is that we may fail to "inject the right OID into the tuple datum" if the types don't match. In case of partitioning, hasoid status must match between the parent and its partitions at all times, so the aforementioned condition is satisfied without requiring that tdtypeid are same. And oid column (if present) is always located at a given position in HeapTuple, so need not map that. Based on the above argument, patch 0006 teaches convert_tuples_by_name() to *optionally* not require tdtypeid of input and output tuple descriptors to be equal. It's implemented by introducing a new argument to convert_tuples_by_name() named 'consider_typeid'. We pass 'false' only for the partitioning cases. (Perhaps, the following should be its own new thread) I noticed that ExecProcessReturning() doesn't work properly after tuple routing (example shows how returning tableoid currently fails but I mention some other issues below): create table p (a int, b int) partition by range (a); create table p1 partition of p for values from (1) to (10); insert into p values (1) returning tableoid::regclass, *; tableoid | a | b --+---+--- -| 1 | (1 row) INSERT 0 1 I tried to fix that in 0007 to get: insert into p values (1) returning tableoid::regclass, *; tableoid | a | b --+---+--- p| 1 | (1 row) INSERT 0 1 But I think it *may* be wrong to return the root table OID for tuples inserted into leaf partitions, because with select we get partition OIDs: select tableoid::regclass, * from p; tableoid | a | b --+---+--- p1 | 1 | (1 row) If so, that means we should build the projection info (corresponding to the returning list) for each target partition somehow. ISTM, that's going to have to be done within the planner by appropriate inheritance translation of the original returning targetlist. Thanks, Amit >From 89f8740195189cc77391bdb844f5092c0440f061 Mon Sep 17 00:00:00 2001 From: amit Date: Mon, 26 Dec 2016 11:53:19 +0900 Subject: [PATCH 1/7] Allocate partition_tuple_slot in respective nodes ...instead of making it part of EState and its tuple table. Respective nodes means ModifyTableState and CopyState for now. Reported by: n/a Patch by: Amit Langote Reports: n/a --- src/backend/commands/copy.c| 30 +- src/backend/executor/execMain.c| 12 src/backend/executor/nodeModify
Re: [HACKERS] ALTER TABLE parent SET WITHOUT OIDS and the oid column
On 2016/12/26 19:06, Amit Langote wrote: > I suspect the following is a bug: A better subject line could be: "ALTER TABLE INHERIT and the oid column" Thanks, Amit -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] ALTER TABLE parent SET WITHOUT OIDS and the oid column
I suspect the following is a bug:
create table foo (a int) with oids;
CREATE TABLE
create table bar (a int);
CREATE TABLE
alter table bar inherit foo;
ERROR: table "bar" without OIDs cannot inherit from table "foo" with OIDs
alter table bar set with oids;
ALTER TABLE
alter table bar inherit foo;
ALTER TABLE
alter table foo set without oids;
ERROR: relation 16551 has non-inherited attribute "oid"
Because:
select attinhcount from pg_attribute where attrelid = 'bar'::regclass and
attname = 'oid';
attinhcount
-
0
(1 row)
Which also means "oid" can be safely dropped from bar breaking the
invariant that if the parent table has oid column, its child tables must too:
alter table bar drop oid; -- or, alter table bar set without oids;
ALTER TABLE
Attached patches modifies MergeAttributesIntoExisting() such that we
increment attinhcount not only for user attributes, but also for the oid
system column if one exists.
Thoughts?
Thanks,
Amit
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index a7ac85e7ab..225a1fe9a1 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -10901,6 +10901,30 @@ MergeAttributesIntoExisting(Relation child_rel, Relation parent_rel)
}
}
+ /*
+ * Must update the child's pg_attribute entry for the "oid" column,
+ * if any.
+ */
+ if (tupleDesc->tdhasoid)
+ {
+ Form_pg_attribute childatt;
+
+ tuple = SearchSysCacheCopyAttName(RelationGetRelid(child_rel), "oid");
+ Assert(HeapTupleIsValid(tuple));
+ childatt = (Form_pg_attribute) GETSTRUCT(tuple);
+ childatt->attinhcount++;
+ /* See the comment above. */
+ if (child_is_partition)
+ {
+ Assert(childatt->attinhcount == 1);
+ childatt->attislocal = false;
+ }
+
+ simple_heap_update(attrrel, &tuple->t_self, tuple);
+ CatalogUpdateIndexes(attrrel, tuple);
+ heap_freetuple(tuple);
+ }
+
heap_close(attrrel, RowExclusiveLock);
}
diff --git a/src/test/regress/expected/inherit.out b/src/test/regress/expected/inherit.out
index 38ea8e86f3..d1d20167c6 100644
--- a/src/test/regress/expected/inherit.out
+++ b/src/test/regress/expected/inherit.out
@@ -1814,3 +1814,28 @@ drop cascades to table part_40_inf
drop cascades to table part_40_inf_ab
drop cascades to table part_40_inf_cd
drop cascades to table part_40_inf_null
+-- check that oid column is handled properly during alter table inherit
+create table _parent (a int) with oids;
+create table _child (a int) with oids;
+alter table _child inherit _parent;
+-- fail
+alter table _child set without oids;
+ERROR: cannot drop inherited column "oid"
+-- should return 1
+select attinhcount from pg_attribute where attrelid = '_child'::regclass and attname = 'oid';
+ attinhcount
+-
+ 1
+(1 row)
+
+alter table _parent set without oids;
+-- should return 0
+select attinhcount from pg_attribute where attrelid = '_child'::regclass and attname = 'oid';
+ attinhcount
+-
+ 0
+(1 row)
+
+-- cleanup
+drop table _parent cascade;
+NOTICE: drop cascades to table _child
diff --git a/src/test/regress/sql/inherit.sql b/src/test/regress/sql/inherit.sql
index e22a14ebda..01e04169e8 100644
--- a/src/test/regress/sql/inherit.sql
+++ b/src/test/regress/sql/inherit.sql
@@ -588,3 +588,18 @@ explain (costs off) select * from range_list_parted where a >= 30;
drop table list_parted cascade;
drop table range_list_parted cascade;
+
+-- check that oid column is handled properly during alter table inherit
+create table _parent (a int) with oids;
+create table _child (a int) with oids;
+alter table _child inherit _parent;
+-- fail
+alter table _child set without oids;
+-- should return 1
+select attinhcount from pg_attribute where attrelid = '_child'::regclass and attname = 'oid';
+alter table _parent set without oids;
+-- should return 0
+select attinhcount from pg_attribute where attrelid = '_child'::regclass and attname = 'oid';
+
+-- cleanup
+drop table _parent cascade;
--
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] proposal: session server side variables
2016-12-26 10:54 GMT+01:00 Pavel Stehule : > Hi > > >> >> In both case, the syntax should be nice and elegant... i.e. not only >> based on functions, probably it should use some prefix convention (@, $)... >> For the light weight option. >> >> DECLARE @someday DATE [ = ] [visibility restriction?]; >> ... then use @now as a possible value anywhere, which will be >> substituted quite early in the execution process, before planning. >> -- update a variable value: >> [SET, ASSIGN, ... nothing?] @someday = ; >> >> Ok, that is basically more or less the mysql syntax, too bad, but I think >> it makes sense for a lightweight object which should not look like a >> database object at all to avoid confusion. >> >> As far as implementation is concerned, I would use a TEXT to TEXT hash >> table, and implicit cast the result when substituting. >> >> @var -> 'text value of var'::type_it_was_declared_with >> > > We are talking about two different features (although the part of name can > be same): > > you are talk about light session variables like MSSQL or MySQL (with same > syntax), I am talking about secure session variables like Oracle package > variables (with similar access syntax). > > Theoretically, there can be implemented both - but cannot be implemented > together. Its are partially different features. My proposal is clearly > related to analogy with Oracle package variables and should to help to > people who does migration from Oracle, or who writing application in Oracle > style - database first, almost logic in database. > > I have two important reasons why I insist on pg_class base. > > 1. security .. it is really fundamental part > Dynamic created variables (like MySQL) cannot be safe - anybody can create variables with self preferred visibility. > 2. possibility to static check by plpgsql_check - without entry in > pg_class (or other catalogue table) I have not any valid information about > type, existence of any variable. > > Although I am not supporter (due possible issues with plpgsql_checks) of > MySQL or MSSQL style variables I am not strongly against this > implementation with same syntax. But it is different feature, with > different benefits and costs. > > I didn't proposed the packages (and package variables) due issues in > multilingual PostgreSQL environment and because it is redundant to > PostgreSQL schemas. Instead I proposed >>secure global session variables<< > (global like global temporary tables). > > Currently light session variables can be implemented as not big extension. > Secure session variables depends on pg_class internals. > > I am not sure if we need a special symbols - it is traditional only. > Set/Get functions can do same work - years we use same technique for > sequences. Setter function is simply. Currently is impossible to write > elegant getter function - because the analyzer has limited work with "any" > returning functions. > > Can be nice to have special hook for functions that returns "any" to push > there some other external informations. > > Regards > > Pavel > > > > >> >> -- >> Fabien. >> > >
Re: [HACKERS] proposal: session server side variables
Hi > > In both case, the syntax should be nice and elegant... i.e. not only based > on functions, probably it should use some prefix convention (@, $)... > For the light weight option. > > DECLARE @someday DATE [ = ] [visibility restriction?]; > ... then use @now as a possible value anywhere, which will be > substituted quite early in the execution process, before planning. > -- update a variable value: > [SET, ASSIGN, ... nothing?] @someday = ; > > Ok, that is basically more or less the mysql syntax, too bad, but I think > it makes sense for a lightweight object which should not look like a > database object at all to avoid confusion. > > As far as implementation is concerned, I would use a TEXT to TEXT hash > table, and implicit cast the result when substituting. > > @var -> 'text value of var'::type_it_was_declared_with > We are talking about two different features (although the part of name can be same): you are talk about light session variables like MSSQL or MySQL (with same syntax), I am talking about secure session variables like Oracle package variables (with similar access syntax). Theoretically, there can be implemented both - but cannot be implemented together. Its are partially different features. My proposal is clearly related to analogy with Oracle package variables and should to help to people who does migration from Oracle, or who writing application in Oracle style - database first, almost logic in database. I have two important reasons why I insist on pg_class base. 1. security .. it is really fundamental part 2. possibility to static check by plpgsql_check - without entry in pg_class (or other catalogue table) I have not any valid information about type, existence of any variable. Although I am not supporter (due possible issues with plpgsql_checks) of MySQL or MSSQL style variables I am not strongly against this implementation with same syntax. But it is different feature, with different benefits and costs. I didn't proposed the packages (and package variables) due issues in multilingual PostgreSQL environment and because it is redundant to PostgreSQL schemas. Instead I proposed >>secure global session variables<< (global like global temporary tables). Currently light session variables can be implemented as not big extension. Secure session variables depends on pg_class internals. I am not sure if we need a special symbols - it is traditional only. Set/Get functions can do same work - years we use same technique for sequences. Setter function is simply. Currently is impossible to write elegant getter function - because the analyzer has limited work with "any" returning functions. Can be nice to have special hook for functions that returns "any" to push there some other external informations. Regards Pavel > > -- > Fabien. >
Re: [HACKERS] Parallel bitmap heap scan
On Wed, Dec 21, 2016 at 3:23 PM, Andres Freund wrote:
Sorry for the delayed response.
> If we go there, it seems better to also wrap the memory context based approach
> in the allocator.
One option is we can keep default allocator in the simple hash, and if
the caller doesn't supply any functions we can use default functions
as I have shown below.
+/* default allocator function */
+static void *
+SH_DEFAULT_ALLOC(Size size, void *args)
+{
+ MemoryContext context = (MemoryContext) args;
+
+ return MemoryContextAllocExtended(context, size,
+ MCXT_ALLOC_HUGE | MCXT_ALLOC_ZERO);
+}
+
+/* default allocator function */
+static void
+SH_DEFAULT_FREE(void *pointer, void *args)
+{
+ pfree(pointer);
+}
+
+SH_CREATE(MemoryContext ctx, uint32 nelements, SH_ALLOCATOR *alloc)
{
- SH_TYPE*tb;
- uint64 size;
+ SH_TYPE *tb;
+ uint64 size;
tb = MemoryContextAllocZero(ctx, sizeof(SH_TYPE));
tb->ctx = ctx;
@@ -294,9 +336,18 @@ SH_CREATE(MemoryContext ctx, uint32 nelements)
SH_COMPUTE_PARAMETERS(tb, size);
- tb->data = MemoryContextAllocExtended(tb->ctx,
- sizeof(SH_ELEMENT_TYPE) * tb->size,
- MCXT_ALLOC_HUGE | MCXT_ALLOC_ZERO);
+ if (!alloc)
+ {
+ tb->alloc = palloc(sizeof(SH_ALLOCATOR));
+ tb->alloc->HashAlloc = SH_DEFAULT_ALLOC;
+ tb->alloc->HashFree = SH_DEFAULT_FREE;
+ tb->alloc->args = tb->ctx;
+ }
+ else
+ tb->alloc = alloc;
+
+ tb->data = tb->alloc->HashAlloc(sizeof(SH_ELEMENT_TYPE) * tb->size,
+ tb->alloc->args);
Other the another option is, that we can always make caller to provide
an allocator. But this way every new user for simple hash need to take
care of having allocator.
What is your opinion?
>This also needs docs, including a warning that just
> using an allocator in shared memory does *NOT* allow the hash table to be
> used in shared memory in the general case.
Make sense.
--
Regards,
Dilip Kumar
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] Protect syscache from bloating with negative cache entries
At Wed, 21 Dec 2016 10:21:09 +0900 (Tokyo Standard Time), Kyotaro HORIGUCHI wrote in <20161221.102109.51106943.horiguchi.kyot...@lab.ntt.co.jp> > At Tue, 20 Dec 2016 15:10:21 -0500, Tom Lane wrote in > <23492.1482264...@sss.pgh.pa.us> > > The bigger picture here though is that we used to have limits on syscache > > size, and we got rid of them (commit 8b9bc234a, see also > > https://www.postgresql.org/message-id/flat/5141.1150327541%40sss.pgh.pa.us) > > not only because of the problem you mentioned about performance falling > > off a cliff once the working-set size exceeded the arbitrary limit, but > > also because enforcing the limit added significant overhead --- and did so > > whether or not you got any benefit from it, ie even if the limit is never > > reached. Maybe the present patch avoids imposing a pile of overhead in > > situations where no pruning is needed, but it doesn't really look very > > promising from that angle in a quick once-over. > > Indeed. As mentioned in the mail at the beginning of this thread, > it hits the whole-cache scanning if at least one negative cache > exists even it is not in a relation with the target relid, and it > can be significantly long on a fat cache. > > Lists of negative entries like CatCacheList would help but needs > additional memeory. > > > BTW, I don't see the point of the second patch at all? Surely, if > > an object is deleted or updated, we already have code that flushes > > related catcache entries. Otherwise the caches would deliver wrong > > data. > > Maybe you take the patch wrongly. Negative entires won't be > flushed by any means. Deletion of a namespace causes cascaded > object deletion according to dependency then finaly goes to > non-neative cache invalidation. But a removal of *negative > entries* in RELNAMENSP won't happen. > > The test script for the case (gen2.pl) does the following thing, > > CREATE SCHEMA foo; > SELECT * FROM foo.invalid; > DROP SCHEMA foo; > > Removing the schema foo leaves a negative cache entry for > 'foo.invalid' in RELNAMENSP. > > However, I'm not sure the above situation happens so frequent > that it is worthwhile to amend. Since 1753b1b conflicts this patch, I rebased this onto the current master HEAD. I'll register this to the next CF. The points of discussion are the following, I think. 1. The first patch seems working well. It costs the time to scan the whole of a catcache that have negative entries for other reloids. However, such negative entries are created by rather unusual usages. Accesing to undefined columns, and accessing columns on which no statistics have created. The whole-catcache scan occurs on ATTNAME, ATTNUM and STATRELATTINH for every invalidation of a relcache entry. 2. The second patch also works, but flushing negative entries by hash values is inefficient. It scans the bucket corresponding to given hash value for OIDs, then flushing negative entries iterating over all the collected OIDs. So this costs more time than 1 and flushes involving entries that is not necessary to be removed. If this feature is valuable but such side effects are not acceptable, new invalidation category based on cacheid-oid pair would be needed. regards, -- Kyotaro Horiguchi NTT Open Source Software Center >From ee0cc13f70d79f23ec9507cf977228bba091bc49 Mon Sep 17 00:00:00 2001 From: Kyotaro Horiguchi Date: Thu, 15 Dec 2016 17:43:03 +0900 Subject: [PATCH 1/2] Cleanup negative cache of pg_statistic when dropping a relation. Accessing columns that don't have statistics causes leaves negative entries in catcache for pg_statstic, but there's no chance to remove them. Especially when repeatedly creating then dropping temporary tables bloats catcache so much that memory pressure can be significant. This patch removes negative entries in STATRELATTINH, ATTNAME and ATTNUM when corresponding relation is dropped. --- src/backend/utils/cache/catcache.c | 57 +++- src/backend/utils/cache/syscache.c | 277 +++-- src/include/utils/catcache.h | 3 + src/include/utils/syscache.h | 2 + 4 files changed, 265 insertions(+), 74 deletions(-) diff --git a/src/backend/utils/cache/catcache.c b/src/backend/utils/cache/catcache.c index 6016d19..c1d9d2f 100644 --- a/src/backend/utils/cache/catcache.c +++ b/src/backend/utils/cache/catcache.c @@ -304,10 +304,11 @@ CatCachePrintStats(int code, Datum arg) if (cache->cc_ntup == 0 && cache->cc_searches == 0) continue; /* don't print unused caches */ - elog(DEBUG2, "catcache %s/%u: %d tup, %ld srch, %ld+%ld=%ld hits, %ld+%ld=%ld loads, %ld invals, %ld lsrch, %ld lhits", + elog(DEBUG2, "catcache %s/%u: %d tup, %d negtup, %ld srch, %ld+%ld=%ld hits, %ld+%ld=%ld loads, %ld invals, %ld lsrch, %ld lhits", cache->cc_relname, cache->cc_indexoid, cache->cc_ntup, + cache->cc_nnegtup, cache->cc_searches, cache->cc_hits, cache->cc_neg_hits, @@ -374,6 +
Re: [HACKERS] proposal: session server side variables
Hello Pavel, On Sat, 24 Dec 2016, Pavel Stehule wrote: Maybe you could consider removing the part of the message that you are not responding to, so that it would be easier for the reader to see your answers and comments. Hmmm. So I understand that you would like to do something like: - call a secure function which sets a session variable with restricted permissions - do some things which cannot access or change the variable - call another secure function which can access, update, remove the variable... I'm still not clear with your use case. Did I read you correctly? ISTM that the above use case could be managed with insert/update/delete in a table with chosen permissions from the functions... Yep, but if you need persistant and transactional then probably you can accept less performant... When you accept less performance, then you can use temporary tables. You can easy wrap it by few polymorphic functions. Probably. This is probably true as well from what I understood from your use case. The namespace issue is unclear to me. Would a variable name clash with a table name? It should if you want to be able write "SELECT stuff FROM variablename", which may or may not be a good idea. It is based on history and experience - one fundamental issue of languages for stored procedures is a conflict of variables and SQL identifiers. I agree that this is a pain, which could be solved by using a prefix, say $ for instance. When variables are based on pg_class, there are not possibility to any new conflict. If variables are based on pg_class, ISTM that they will cost anyway. To sum up my current opinion, taking into accounts your use case and Tom & Jim argments about performance, I think that variables should be either: - full-featured database objects well integrated in the database logic: CREATE/ALTER/DROP, in pg_class, subject to standard permissions, constraints, transactions, possibly persistent... Basically like a one-row table, although the implementation should be more efficient, I agree. ** OR ** - very light-weight, a simple server process key-value store, which would not use CREATE/ALTER/DROP which suggest otherwise, they would not be subject to permissions nor transactions nor persistence but die with the session, goodbye. A possible concession to permissions would be to have a per-role store, and/or some visibility/accessibility declaration at creation time, but certainly not GRANT/RESTORE syntax which suggest a database persistent object. I'm very reserved about anything in between these two options, which looks like a database object but is not really one, so I think that it create confusion. In both case, the syntax should be nice and elegant... i.e. not only based on functions, probably it should use some prefix convention (@, $)... For the light weight option. DECLARE @someday DATE [ = ] [visibility restriction?]; ... then use @now as a possible value anywhere, which will be substituted quite early in the execution process, before planning. -- update a variable value: [SET, ASSIGN, ... nothing?] @someday = ; Ok, that is basically more or less the mysql syntax, too bad, but I think it makes sense for a lightweight object which should not look like a database object at all to avoid confusion. As far as implementation is concerned, I would use a TEXT to TEXT hash table, and implicit cast the result when substituting. @var -> 'text value of var'::type_it_was_declared_with -- Fabien. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
