Re: [HACKERS] The behavior of CheckRequiredParameterValues()
On Wed, Mar 5, 2014 at 12:07 PM, Amit Langote amitlangot...@gmail.com wrote: On Wed, Mar 5, 2014 at 2:09 AM, Sawada Masahiko sawada.m...@gmail.com wrote: xlog.c:6177 if (ControlFile-wal_level WAL_LEVEL_HOT_STANDBY) ereport(ERROR, (errmsg(hot standby is not possible because wal_level was not So we have to start and stop standby server with changed wal_level(i.g., hot_standby) if we want to enable hot standby. In this case, I think that the standby server didn't need to confirm wal_level value of ControlFile. I think that it should confirm value which is written in postgreql.conf. I think checking it from the control file on a standby in recovery means that we should confirm if the *wal_level with which the WAL was generated* is sufficient to now become a hot standby after recovery finishes. Sorry, should have said: *become a hot standby after recovery reaches a consistent state -- Amit -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: Custom Scan APIs (Re: [HACKERS] Custom Plan node)
Tom, thanks for your detailed comments. I apologize for not having paid much attention to this thread so far. It kept getting stuck on my to look at later queue. Anyway, I've taken a preliminary look at the v7 patch now. While the patch seems roughly along the lines of what we talked about last PGCon, I share Stephen's unease about a lot of the details. It's not entirely clear that these hooks are really good for anything, and it's even less clear what APIs the hook functions should be expected to depend on. I really do not like the approach embodied in the later patches of oh, we'll just expose whatever static planner functions seem convenient. That's not an approach that's available when doing actual external development of an extension, and even if it were that doesn't make it a good idea. The larger the exposed surface of functions the harder it is to know what's safe to change. Hmm. It needs a clear reasoning to expose the function rather than its convenience. Anyway, on to specifics: * Please drop the whole register_custom_provider/get_custom_provider API. There is no reason other than debugging for a provider to have a name at all, and if we expect providers to have unique names then that creates a collision risk for independently-created extensions. AFAICS, it's sufficient for a function hooked into one of the add-a-path hooks to include a pointer to a struct-of-function-pointers in the Path object it returns, and similarly the CustomScan Plan object can contain a pointer inserted when it's created. I don't object to having a name field in the function pointer structs for debugging reasons, but I don't want any lookups being done on it. One thing I was worrying about is how copyObject() and nodeToString() support set of function pointer tables around custom-scan node, however, you suggested to change the assumption here. So, I think these functions become unnecessary. * The function-struct pointers should be marked const in the referencing nodes, to indicate that the core code won't be modifying or copying them. In practice they'd probably be statically allocated constants in the extensions anyway. OK, * The patch does lots of violence to the separation between planner and executor, starting with the decision to include nodes/relation.h in executor.h. That will not do at all. I see that you did that because you wanted to make ExecSupportsMarkRestore take a Path, but we need some other answer. One slightly grotty answer is to invent two different customscan Plan types, one that supports mark/restore and one that doesn't, so that ExecSupportsMarkRestore could still just look at the Plan type tag. (BTW, path-pathtype is supposed to contain the node tag of the Plan node that the path would produce. Putting T_CustomPath in it is entirely tone-deaf.) Another way would be to remove ExecSupportsMarkRestore in favor of some new function in the planner; but it's good to keep it in execAmi.c since that has other knowledge of which plan types support mark/restore. OK, I'll add one derivative node delivertive plan node type, CustomScanMarkRestore for instance. Probably, it shall be populated on the create_customscan_plan() according to the flag being set on the CustomPath. * More generally, I'm not convinced that exactly one Path type and exactly one Plan type is going to get us very far. It seems rather ugly to use the same Plan type for both scan and join nodes, and what will happen if somebody wants to build a custom Append node, or something else that has neither zero nor two subplans? In the previous discussion, CustomJoin will be nonsense because we know limited number of join algorithms: nest-loop, hash-join and merge-join, unlike variation of logic to scan relations. Also, IIUC, someone didn't want to add custom- something node types for each built-in types. So, we concluded to put CustomScan node to replace built-in join / scan at that time. Regarding to the Append node, it probably needs to be enhanced to have list of subplans on CustomScan, or add individual CustomAppend node, or opaque CustomPlan may be sufficient if it handles EXPLAIN by itself. * nodeCustom.h is being completely abused. That should only export the functions in nodeCustom.c, which are going to be pretty much one-liners anyway. The right place to put the function pointer struct definitions is someplace else. I'd be inclined to start by separating the function pointers into two structs, one for functions needed for a Path and one for functions needed for a Plan, so that you don't have this problem of having to import everything the planner knows into an executor header or vice versa. Most likely you could just put the Path function pointer struct declaration next to CustomPath in relation.h, and the one for Plans next to CustomPlan (or the variants thereof) in plannodes.h. Yes. I didn't have clear idea where we should put the definition of
Re: [HACKERS] Hot standby doesn't come up on some situation.
Hello,
After all, I have confirmed that this fixes the problem on crash
recovery of hot-standby botfor 9.3 and HEAD and no problem was
found except unreadability :(
By the way, I moderately want to fix an assertion message to a
ordinary one. Details are below.
The server stops with following message during restarting after
crash requesting archive recovery when the WAL has been produced
with the wal_level below WAL_LEVEL_HOT_STANDBY.
| TRAP: FailedAssertion(!(((oldestActiveXID) != ((TransactionId) 0))), File:
xlog.c, Line: 6799)
| LOG: startup process (PID 7270) was terminated by signal 6: Aborted
Surely this is the consequence of illegal operation but I think
it is also not a issue of assertion - which fires on something
wrong in design or quite rare cases(this case ?). So it might be
better to show message as below on the case.
| FATAL: Checkpoint doesn't have valid oldest active transaction id
| HINT: Reading WAL might have been written under insufficient wal_level.
This could do in this way,
==
diff --git a/src/backend/access/transam/xlog.c
b/src/backend/access/transam/xlog.c
index e3d5e10..bb6922a 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -6789,7 +6789,13 @@ StartupXLOG(void)
if (wasShutdown)
oldestActiveXID =
PrescanPreparedTransactions(xids, nxids);
else
+ {
oldestActiveXID = checkPoint.oldestActiveXid;
+ if (!TransactionIdIsValid(oldestActiveXID))
+ ereport(FATAL,
+ (errmsg(Checkpoint
doesn't have valid oldest active transaction id),
+errhint(Reading WAL
might have been written under insufficient wal_level.)));
+ }
Assert(TransactionIdIsValid(oldestActiveXID));
/* Tell procarray about the range of xids it has to
deal with */
=
What do you think about this? Feel free dumping this if you feel
negative on 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
Re: [HACKERS] Row-security on updatable s.b. views
On 2014-03-05 04:02, Craig Ringer wrote: On 03/04/2014 09:41 PM, Yeb Havinga wrote: On 04/03/14 02:36, Craig Ringer wrote: I've pushed an update to the branch with the fix for varno handling. Thanks. It's tagged rls-9.4-upd-sb-views-v8 . I've almost run out of time to spend on row security for this commitfest, unfortunately. I'm putting a blog together with a current status update. Frustrating, as it's coming together now. Open issues include: - Passing plan inval items from rewriter into planner - COPY support pending - Clear syntax in DDL Most of the rest are solved; it's actually looking pretty good. Hi Craig, I've tested the results from the minirim.sql that was posted earlier, and the v8 gives the same results as v4 :-) Good to hear. The main known issue remaining is plan invalidation. Row security needs to create PlanInvalItems during _rewrite_ and also needs to set a PlannerGlobal field for the user ID the plan depends on. If it fails to do this then the superuser will sometimes run queries and have row-security applied, non-superusers might skip row security when it should be applied. This seems to be the cause of foreign key check issues I've observed, too. That's not trivial, because right now the rewriter only returns a Query node. It doesn't have anywhere to stash information that's global across the whole query, and adding fields to Query for the purpose doesn't seem ideal, since it's also used for subqueries, and in the planner. I looked up the Query structure and steps of e.g. exec_simple_query(), but ISTM that Query would be the place to store a used id. After all it is meta data about the query. Duplication of this information in the presence of subqueries seems less ugly to me than trying to evade duplication by wrapping a structure around a query list. Maybe a naive thought, but shouldn't all plans that include a table with an RLS clause be invalidated when the session role switches, regardless of which users from and to? I've also got some concerns about the user visible API; I'm not sure it makes sense to set row security policy for row reads per-command in PostgreSQL, since we have the RETURNING clause. Read-side policy should just be FOR READ. Hmm but FOR READ would mean new keywords, and SELECT is also a concept known to users. I didn't find the api problematic to understand, on the contrary. It might be an idea to add the SELECT RLS clause for DML queries that contain a RETURNING clause. regards, Yeb -- 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] ALTER TABLE lock strength reduction patch is unsafe
On 4 March 2014 21:37, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: On Tue, Mar 4, 2014 at 2:39 PM, Simon Riggs si...@2ndquadrant.com wrote: Your earlier claim that the dump is inconsistent just isn't accurate. We now have MVCC catalogs, so any dump is going to see a perfectly consistent set of data plus DDL. OK the catalogs may change AFTER the snapshot was taken for the dump, but then so can the data change - that's just MVCC. Unfortunately, this isn't correct. The MVCC snapshots taken for catalog scans are instantaneous; that is, we take a new, current snapshot for each catalog scan. If all of the ruleutils.c stuff were using the transaction snapshot rather than instantaneous snapshots, this would be right. But as has been previously discussed, that's not the case. Yeah. And that's *necessary* for catalog lookups in a normally functioning backend, because we have to see latest data (eg, it wouldn't do for a backend to fail to enforce a just-added CHECK constraint because it was committed after the backend's transaction started). OK, thanks for explaining. A valuable point to note for us all. However, it seems possible that we could have a mode in which a read-only session did all its catalog fetches according to the transaction snapshot. That would get us to a situation where the backend-internal lookups that ruleutils relies on would give the same answers as queries done by pg_dump. Robert's work on getting rid of SnapshotNow has probably moved that much closer than it was before, but it's still not exactly a trivial patch. Meanwhile, Andres claimed upthread that none of the currently-proposed reduced-lock ALTER commands affect data that pg_dump is using ruleutils to fetch. If that's the case, then maybe this is a problem that we can punt till later. I've not gone through the list to verify it though. So that returns us to solving the catalog consistency problem in pg_dump and similar applications. We could (1) change the lock taken by pg_dump to be ShareUpdateExclusive. As discussed, this would be optional. (Trivial implementation) The catalog accesses are all in a rather isolated piece of code in pg_dump and run for a short period. That allows us to consider locking *always* at ShareUpdateExclusive but only for the period of catalog access and then release the higher level lock before transaction end. Since pg_dump is a client program any action we take to resolve this would need to be done in a user accessible way. That is acceptable since there may be other user programs that wish/need to read a consistent view of the definition of a table. This can be implemented in a few ways: (2) Implement a server function that allows you to lock a table for a short duration. e.g. pg_lock_catalog(Oid) and pg_unlock_catalog(Oid). We can already do this in server-side code, so this is simply a matter of exposing that same functionality for users. (3) A new variant of the LOCK command: LOCK CATALOG FOR tablename IN lock mode MODE NOWAIT, which then would have a matching UNLOCK CATALOG FOR tablename command. This is just a sugar coated version of (2). (4) Implement functions to suspend invalidation message handling for a short period. That's basically the same as (2) in profile. My feeling is that sounds rather dangerous and not something I'd want to go near now in in the future. We tried to avoid locking the catalog some years back, which is how we went off down this MVCC catalog access, which now seems to have been something of a red-shifted herring. ISTM that the user would need to specifically request a consistent catalog. Using (2) in pg_dump is pretty easy - patch attached. So we can solve this problem completely in about another 1 hour of work, so I suggest we implement (2) and be done. Happy to document this in a new subsection of docs to describe how to dump a consistent view of a database object from a user application. -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services lock_catalog_for_pgdump.v1.patch Description: Binary data -- 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] Review: Patch FORCE_NULL option for copy COPY in CSV mode
On Wed, Mar 5, 2014 at 7:44 AM, Andrew Dunstan and...@dunslane.net wrote: I have picked this up and committed the patch. Thanks to all. Sorry for coming after the battle, but while looking at what has been committed I noticed that copy2.sql is actually doing twice in a row the same test: COPY forcetest (a, b, c) FROM STDIN WITH (FORMAT csv, FORCE_NOT_NULL(b), FORCE_NULL(c)); 1,, \. -- should succeed with no effect (b remains an empty string, c remains NULL) COPY forcetest (a, b, c) FROM STDIN WITH (FORMAT csv, FORCE_NOT_NULL(b), FORCE_NULL(c)); 2,, \. See? For both tests the quotes are placed on the same column, the 3rd. I think that one of them should be like that, with the quotes on the 2nd column = 2,, The attached patch corrects that... and a misplaced comment. Regards, -- Michael diff --git a/src/test/regress/expected/copy2.out b/src/test/regress/expected/copy2.out index 76dea28..e8fb3d1 100644 --- a/src/test/regress/expected/copy2.out +++ b/src/test/regress/expected/copy2.out @@ -383,7 +383,6 @@ SELECT * FROM vistest; (2 rows) -- Test FORCE_NOT_NULL and FORCE_NULL options --- should succeed with b set to an empty string and c set to NULL CREATE TEMP TABLE forcetest ( a INT NOT NULL, b TEXT NOT NULL, @@ -392,6 +391,7 @@ CREATE TEMP TABLE forcetest ( e TEXT ); \pset null NULL +-- should succeed with b set to an empty string and c set to NULL BEGIN; COPY forcetest (a, b, c) FROM STDIN WITH (FORMAT csv, FORCE_NOT_NULL(b), FORCE_NULL(c)); COMMIT; diff --git a/src/test/regress/sql/copy2.sql b/src/test/regress/sql/copy2.sql index e2be21f..63332bd 100644 --- a/src/test/regress/sql/copy2.sql +++ b/src/test/regress/sql/copy2.sql @@ -271,7 +271,6 @@ SELECT * FROM vistest; COMMIT; SELECT * FROM vistest; -- Test FORCE_NOT_NULL and FORCE_NULL options --- should succeed with b set to an empty string and c set to NULL CREATE TEMP TABLE forcetest ( a INT NOT NULL, b TEXT NOT NULL, @@ -280,6 +279,7 @@ CREATE TEMP TABLE forcetest ( e TEXT ); \pset null NULL +-- should succeed with b set to an empty string and c set to NULL BEGIN; COPY forcetest (a, b, c) FROM STDIN WITH (FORMAT csv, FORCE_NOT_NULL(b), FORCE_NULL(c)); 1,, @@ -289,7 +289,7 @@ SELECT b, c FROM forcetest WHERE a = 1; -- should succeed with no effect (b remains an empty string, c remains NULL) BEGIN; COPY forcetest (a, b, c) FROM STDIN WITH (FORMAT csv, FORCE_NOT_NULL(b), FORCE_NULL(c)); -2,, +2,, \. COMMIT; SELECT b, c FROM forcetest WHERE a = 2; -- 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] Hot standby doesn't come up on some situation.
On 03/05/2014 10:51 AM, Kyotaro HORIGUCHI wrote:
Hello,
After all, I have confirmed that this fixes the problem on crash
recovery of hot-standby botfor 9.3 and HEAD and no problem was
found except unreadability :(
Ok, committed. Thanks!
Any concrete suggestions about the readability? Is there some particular
spot that needs clarifying?
By the way, I moderately want to fix an assertion message to a
ordinary one. Details are below.
The server stops with following message during restarting after
crash requesting archive recovery when the WAL has been produced
with the wal_level below WAL_LEVEL_HOT_STANDBY.
| TRAP: FailedAssertion(!(((oldestActiveXID) != ((TransactionId) 0))), File:
xlog.c, Line: 6799)
| LOG: startup process (PID 7270) was terminated by signal 6: Aborted
Surely this is the consequence of illegal operation but I think
it is also not a issue of assertion - which fires on something
wrong in design or quite rare cases(this case ?).
Ah, I see. Yes, that's definitely a bug. If you don't hit the assertion,
because the oldestActiveXID is set in the checkpoint record even though
wal_level is 'archive', or if you simply have assertions disabled, the
system will start up in hot standby mode even though it's not safe.
So it might be
better to show message as below on the case.
| FATAL: Checkpoint doesn't have valid oldest active transaction id
| HINT: Reading WAL might have been written under insufficient wal_level.
This could do in this way,
==
diff --git a/src/backend/access/transam/xlog.c
b/src/backend/access/transam/xlog.c
index e3d5e10..bb6922a 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -6789,7 +6789,13 @@ StartupXLOG(void)
if (wasShutdown)
oldestActiveXID =
PrescanPreparedTransactions(xids, nxids);
else
+ {
oldestActiveXID = checkPoint.oldestActiveXid;
+ if (!TransactionIdIsValid(oldestActiveXID))
+ ereport(FATAL,
+ (errmsg(Checkpoint doesn't
have valid oldest active transaction id),
+errhint(Reading WAL might
have been written under insufficient wal_level.)));
+ }
Assert(TransactionIdIsValid(oldestActiveXID));
/* Tell procarray about the range of xids it has to
deal with */
=
What do you think about this? Feel free dumping this if you feel
negative on this.
Hmm. When I test that with 9.2, oldestActiveXID is not 0, even though
wal_level is 'archive'. So the above patch doesn't fix the whole problem.
The real bug here is that CheckRequiredParameterValues() tests for
InArchiveRecovery, when it should be testing for
ArchiveRecoveryRequested. Otherwise, the checks are not performed when
going through the crash recovery followed by archive recovery. I
should've changed that as part of the commit that added the crash
recovery then archive recovery behavior.
Fixed, thanks for pointing it out!
- Heikki
--
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] Review: Patch FORCE_NULL option for copy COPY in CSV mode
After testing this feature, I noticed that FORCE_NULL and
FORCE_NOT_NULL can both be specified with COPY on the same column.
This does not seem correct. The attached patch adds some more error
handling, and a regression test case for that.
Regards,
--
Michael
diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c
index 70ee7e5..540da91 100644
--- a/src/backend/commands/copy.c
+++ b/src/backend/commands/copy.c
@@ -1457,6 +1457,26 @@ BeginCopy(bool is_from,
}
}
+ /*
+* Check if both force_null and force_not_null are used on the same
+* columns.
+*/
+ if (cstate-force_null cstate-force_notnull)
+ {
+ int i;
+
+ for (i = 0; i num_phys_attrs; i++)
+ {
+ if (cstate-force_notnull_flags[i]
+ cstate-force_null_flags[i])
+ ereport(ERROR,
+ (errcode(ERRCODE_SYNTAX_ERROR),
+errmsg(conflicting or
redundant options),
+errhint(\force_not_null\
and \force_null\ specified for the same column \%s\,
+
NameStr(tupDesc-attrs[i]-attname;
+ }
+ }
+
/* Use client encoding when ENCODING option is not specified. */
if (cstate-file_encoding 0)
cstate-file_encoding = pg_get_client_encoding();
diff --git a/src/test/regress/expected/copy2.out
b/src/test/regress/expected/copy2.out
index 76dea28..5341b09 100644
--- a/src/test/regress/expected/copy2.out
+++ b/src/test/regress/expected/copy2.out
@@ -418,6 +418,12 @@ ERROR: null value in column b violates not-null
constraint
DETAIL: Failing row contains (3, null, , null, null).
CONTEXT: COPY forcetest, line 1: 3,,
ROLLBACK;
+-- FORCE_NULL and FORCE_NOT_NULL cannot be used on the same column
+BEGIN;
+COPY forcetest (a) FROM STDIN WITH (FORMAT csv, FORCE_NULL(a),
FORCE_NOT_NULL(a));
+ERROR: conflicting or redundant options
+HINT: force_not_null and force_null specified for the same column a
+ROLLBACK;
-- should fail with not referenced by COPY error
BEGIN;
COPY forcetest (d, e) FROM STDIN WITH (FORMAT csv, FORCE_NOT_NULL(b));
diff --git a/src/test/regress/sql/copy2.sql b/src/test/regress/sql/copy2.sql
index e2be21f..91dc902 100644
--- a/src/test/regress/sql/copy2.sql
+++ b/src/test/regress/sql/copy2.sql
@@ -299,6 +299,10 @@ COPY forcetest (a, b, c) FROM STDIN WITH (FORMAT csv,
FORCE_NULL(b), FORCE_NOT_N
3,,
\.
ROLLBACK;
+-- FORCE_NULL and FORCE_NOT_NULL cannot be used on the same column
+BEGIN;
+COPY forcetest (a) FROM STDIN WITH (FORMAT csv, FORCE_NULL(a),
FORCE_NOT_NULL(a));
+ROLLBACK;
-- should fail with not referenced by COPY error
BEGIN;
COPY forcetest (d, e) FROM STDIN WITH (FORMAT csv, FORCE_NOT_NULL(b));
--
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] Review: Patch FORCE_NULL option for copy COPY in CSV mode
On 03/05/2014 09:11 AM, Michael Paquier wrote: After testing this feature, I noticed that FORCE_NULL and FORCE_NOT_NULL can both be specified with COPY on the same column. This does not seem correct. The attached patch adds some more error handling, and a regression test case for that. Strictly they are not actually contradictory, since FORCE NULL relates to quoted null strings and FORCE NOT NULL relates to unquoted null strings. Arguably the docs are slightly loose on this point. Still, applying both FORCE NULL and FORCE NOT NULL to the same column would be rather perverse, since it would result in a quoted null string becoming null and an unquoted null string becoming not null. I'd be more inclined just to tighten the docs and maybe expand the regression tests a bit, but I could be persuaded the other way if people think it's worth it. cheers andrew -- 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] Negative Transition Aggregate Functions (WIP)
On Mar4, 2014, at 21:09 , Dean Rasheed dean.a.rash...@gmail.com wrote: On 3 March 2014 23:00, Florian Pflug f...@phlo.org wrote: * In show_windowagg_info(), this calculation looks suspicious to me: double tperrow = winaggstate-aggfwdtrans / (inst-nloops * inst-ntuples); If the node is executed multiple times, aggfwdtrans will be reset in each loop, so the transitions per row figure will be under-estimated. ISTM that if you want to report on this, you'd need aggfwdtrans to be reset once per query, but I'm not sure exactly how to do that. ... Actually, I think it's misleading to only count forward transition function calls, because a call to the inverse transition function still represents a state transition, and is likely to be around the same cost. For a window of size 2, there would not be much advantage to using inverse transition functions, because it would be around 2 transitions per row either way. True. In fact, I pondered whether to avoid using the inverse transition function for windows of 2 rows. In the end, I didn't because I felt that it makes custom aggregates harder to test. On the question of whether to count inverse transition function calls - the idea of the EXPLAIN VERBOSE ANALYZE output isn't really to show the number of state transitions, but rather to show whether the aggregation has O(n) or O(n^2) behaviour. The idea being that a value close to 1 means inverse transition function works as expected, and larger values mean not working so well. Regarding multiple evaluations - I think I based the behaviour on how ntuples works, which also only reports the value of the last evaluation I think. But maybe I'm confused about this. No, it doesn't look like that's correct for multiple loops. Consider this example: ... It turns out that show_windowagg_info() is only called once at the end, with ntuples=100, nloops=4 and aggfwdtrans=100, so it's computing tperrow=100/(4*100)=0.25, which then gets truncated to 0.2. So to get 1, you'd have to use this formula: double tperrow = winaggstate-aggfwdtrans / inst-ntuples; Hm, so I *was* confused - seems I mixed up ntuples (which counts the total number of tuples over all loops) with what we report as rows (i.e. the average number of rows per loop). Thanks for clearing that up! I'm still not convinced that's the most useful thing to report though. Personally, I'd prefer to just see the separate counts, e.g.: - WindowAgg (cost=0.00..22.50 rows=1000 width=4) (actual time=0.019..0.094 rows=25 loops=4) Output: sum(i.i) OVER (?) Forward transitions: 25 Inverse transitions: 25 IMO that gives a clearer picture of what's going on. My problem with this is that if there are multiple aggregates, the numbers don't really count the number of forward and reverse transfer function invocations. What nodeWindowAgg.c really counts is the number of *rows* it has to fetch from the tuple store to perform forward aggregations - the relevant code snippet is if (numaggs_restart 0) winstate-aggfwdtrans += (winstate-aggregatedupto - winstate-frameheadpos); else winstate-aggfwdtrans += (winstate-aggregatedupto - aggregatedupto_nonrestarted); Now we could of course report these figures per aggregate instead of only once per aggregation node. But as I said earlier, my guess is that people usually won't be interested in the precise counts - most likely, what you want to know is how much do the restarts cost me When I added the EXPLAIN stuff, I initially simply reported the number of times nodeWindowAgg has to restart the aggregation. The problem with that approach is that not all restarts carry the same cost. If the frames either don't overlap at all or are identical, restarts don't cause any additional work. And for fixed-length frames (BETWEEN n PRECEEDING AND m FOLLOWING), the performance effects of restarts depends on m-n. Which is why I made it count the number of aggregated input rows instead. Having said that, I' not really 100% happy with the name Transitions Per Row for this - it was simply the best I could come up with that was reasonably short. And I'm certainly open to reporting the absolute count instead of a factor relative to ntuples. If we made it an absolute count, would calling this Aggregated Rows work for you? best regards, Florian Pflug -- 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] jsonb and nested hstore
On Mon, Mar 3, 2014 at 06:59:37PM -0800, Josh Berkus wrote: Realistically, hstore will never go away. I'll bet you a round or two of pints that, if we get both hstore2 and jsonb, within 2 years the users of jsonb will be an order of magnitude more numerous that then users of hstore, but folks out there have apps already built against hstore1 and they're going to keep on the hstore path. In the discussion you haven't apparently caught up on yet, we did discuss moving *hstore* into core to make this whole thing easier. However, that fell afoul of the fact that we currently have no mechanism to move types between extensions and core without breaking upgrades for everyone. So the only reason hstore is still an extension is because of backwards-compatibility. I have read last week's thread on this issue, and it certainly seems we are in a non-ideal situation here. The discussion centered around the split of JSONB in core and hstore in contrib, the reason for some of these decisions, and what can make it into PG 9.4. I would like to take a different approach and explore what we _eventually_ want, then back into what we have and what can be done for 9.4. Basically, it seems we have heirchical hstore and JSONB which are identical except for the input/output syntax. Many are confused how a code split like that works long-term, and whether decisions made for 9.4 might limit future options. There seems to be a basic tension that we can't move hstore into core, must maintain backward-compatibility for hstore, and we want JSONB in core. Long-term, having JSON in core and JSONB in contrib seems quite odd. So, I am going to ask a back-track question and ask why we can't move hstore into core. Is this a problem with the oids of the hstore data type and functions? Is this a pg_upgrade-only problem? Can this be fixed? Yes, I am ignoring what might be possible for 9.4, but I think these questions must be asked if we are going to properly plan for post-9.4 changes. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + -- 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] Row-security on updatable s.b. views
On 03/05/2014 05:25 PM, Yeb Havinga wrote: Maybe a naive thought, but shouldn't all plans that include a table with an RLS clause be invalidated when the session role switches, regardless of which users from and to? Only if the plan is actually accessed when under a different user ID. Consider SECURITY DEFINER functions; you don't want to flush all cached plans just because you ran a SECURITY DEFINER function that doesn't even share any statements with the outer transaction. Anyway, the same issue remains: how to pass the information this plan is user-id specific from rewriter to planner. I've also got some concerns about the user visible API; I'm not sure it makes sense to set row security policy for row reads per-command in PostgreSQL, since we have the RETURNING clause. Read-side policy should just be FOR READ. Hmm but FOR READ would mean new keywords, and SELECT is also a concept known to users. I didn't find the api problematic to understand, on the contrary. Would you expect that FOR SELECT also affects rows you can see to UPDATE, INSERT, or DELETE? Because that's what it would have to mean, really. Otherwise, you could just use `UPDATE thetable SET id = id RETURNING *` (or whatever) to read the rows out if you had UPDATE rights. Or do the same with DELETE. With RETURNING, it doesn't make much sense for different statements to have different read access. Can you think of a case where it'd be reasonable to deny SELECT, but allow someone to see the same rows with `UPDATE ... RETURNING` ? It might be an idea to add the SELECT RLS clause for DML queries that contain a RETURNING clause. That way lies madness: A DML statement that affects *a different set of rows* depending on whether or not it has a RETURNING clause. -- Craig Ringer http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, 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] Review: Patch FORCE_NULL option for copy COPY in CSV mode
2014-03-05 23:27 GMT+09:00 Andrew Dunstan and...@dunslane.net: On 03/05/2014 09:11 AM, Michael Paquier wrote: After testing this feature, I noticed that FORCE_NULL and FORCE_NOT_NULL can both be specified with COPY on the same column. This does not seem correct. The attached patch adds some more error handling, and a regression test case for that. Strictly they are not actually contradictory, since FORCE NULL relates to quoted null strings and FORCE NOT NULL relates to unquoted null strings. Arguably the docs are slightly loose on this point. Still, applying both FORCE NULL and FORCE NOT NULL to the same column would be rather perverse, since it would result in a quoted null string becoming null and an unquoted null string becoming not null. Too frazzled to recall clearly right now, but I think that was the somewhat counterintuitive conclusion I originally came to. I'd be more inclined just to tighten the docs and maybe expand the regression tests a bit, but I could be persuaded the other way if people think it's worth it. Might be worth doing if it's an essentially useless feature, if only to preempt an unending chain of bug reports. Many thanks for everyone's input on this, and apologies for not giving the patch enough rigorous attention. Regards Ian Barwick -- 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] jsonb and nested hstore
On 03/05/2014 09:39 AM, Bruce Momjian wrote: So, I am going to ask a back-track question and ask why we can't move hstore into core. Is this a problem with the oids of the hstore data type and functions? Is this a pg_upgrade-only problem? Can this be fixed? Yes, pg_upgrade is the problem, and no, I can't see how it can be fixed. Builtin types have Oids in a certain range. Non-builtin types have Oids outside that range. If you have a clever way to get over that I'd be all ears, but it seems to me insurmountable right now. A year or two ago I made a suggestion to help avoid such problems in future, but as Josh said it got shot down, and in any case it would not have helped with existing types such as hstore. cheers andrew -- 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] Review: Patch FORCE_NULL option for copy COPY in CSV mode
On Wed, Mar 5, 2014 at 11:37 PM, Ian Lawrence Barwick barw...@gmail.com wrote: 2014-03-05 23:27 GMT+09:00 Andrew Dunstan and...@dunslane.net: On 03/05/2014 09:11 AM, Michael Paquier wrote: After testing this feature, I noticed that FORCE_NULL and FORCE_NOT_NULL can both be specified with COPY on the same column. This does not seem correct. The attached patch adds some more error handling, and a regression test case for that. Strictly they are not actually contradictory, since FORCE NULL relates to quoted null strings and FORCE NOT NULL relates to unquoted null strings. Arguably the docs are slightly loose on this point. Still, applying both FORCE NULL and FORCE NOT NULL to the same column would be rather perverse, since it would result in a quoted null string becoming null and an unquoted null string becoming not null. Too frazzled to recall clearly right now, but I think that was the somewhat counterintuitive conclusion I originally came to. In this case I may be an intuitive guy :), but OK I see your point. So if we specify both this produces the exact opposite as the default, default being an empty string inserted for a quoted empty string and NULL inserted for a non-quoted empty string. So yes I'm fine with a note on the docs about that, and some more regression tests. Btw, if we allow this behavior in COPY, why doesn't file_fdw allow both options to be allowed on the same column for a foreign table? Current behavior of file_fdw seems rather inconsistent with COPY as it stands now. -- 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] Review: Patch FORCE_NULL option for copy COPY in CSV mode
On Wed, Mar 5, 2014 at 11:58 PM, Michael Paquier michael.paqu...@gmail.com wrote: So if we specify both this produces the exact opposite of the default, default being an empty string inserted for a quoted empty string and NULL inserted for a non-quoted empty string. So yes I'm fine with a note on the docs about that, and some more regression tests. For people who did not get this one, here is a short example: =# ¥pset null 'null' Null display (null) is null. =# create table aa (a text); CREATE TABLE =# COPY aa FROM STDIN WITH (FORMAT csv); Enter data to be copied followed by a newline. End with a backslash and a period on a line by itself. \. =# select * from aa; a -- null (2 rows) =# truncate aa; TRUNCATE TABLE Time: 12.149 ms =# COPY aa FROM STDIN WITH (FORMAT csv, FORCE_NULL(a), FORCE_NOT_NULL(a)); Enter data to be copied followed by a newline. End with a backslash and a period on a line by itself. \. Time: 3776.662 ms =# select * from aa; a -- null (2 rows) -- 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] Review: Patch FORCE_NULL option for copy COPY in CSV mode
Andrew Dunstan and...@dunslane.net writes: On 03/05/2014 09:11 AM, Michael Paquier wrote: After testing this feature, I noticed that FORCE_NULL and FORCE_NOT_NULL can both be specified with COPY on the same column. Strictly they are not actually contradictory, since FORCE NULL relates to quoted null strings and FORCE NOT NULL relates to unquoted null strings. Arguably the docs are slightly loose on this point. Still, applying both FORCE NULL and FORCE NOT NULL to the same column would be rather perverse, since it would result in a quoted null string becoming null and an unquoted null string becoming not null. Given the remarkable lack of standardization of CSV output, who's to say that there might not be data sources out there for which this is the desired behavior? It's weird, I agree, but I think throwing an error for the combination is not going to be helpful. It's not like somebody might accidentally write both on the same column. +1 for clarifying the docs, though, more or less in the words you used above. 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: Fwd: [HACKERS] patch: make_timestamp function
Hi
I hope, so this patch fix it
Regards
Pavel
2014-03-04 21:00 GMT+01:00 Pavel Stehule pavel.steh...@gmail.com:
2014-03-04 20:20 GMT+01:00 Alvaro Herrera alvhe...@2ndquadrant.com:
Pavel Stehule escribió:
2014-03-04 19:12 GMT+01:00 Alvaro Herrera alvhe...@2ndquadrant.com:
Pavel Stehule escribió:
Hello
updated version - a precheck is very simple, and I what I tested it
is
enough
Okay, thanks. I pushed it after some more editorialization. I don't
think I broke anything, but please have a look.
It looks well
Coypu is showing a strange failure though:
http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=coypudt=2014-03-04%2018%3A22%3A31
select make_interval(secs := 'inf');
! make_interval
! -
! @ 0.01 secs ago
! (1 row)
I realize that we have some hacks in float4in and float8in to deal with
these portability issues ... Maybe the fix is just take out the test.
I have no idea, how to fix it now and have to leave a office. Tomorrow
I'll try to fix it.
Regards
Pavel
--
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training Services
commit 956685f82b6983ff17e6a39bd386b11f554715a8
Author: Heikki Linnakangas heikki.linnakan...@iki.fi
Date: Wed Mar 5 14:41:55 2014 +0200
Do wal_level and hot standby checks when doing crash-then-archive recovery.
CheckRequiredParameterValues() should perform the checks if archive recovery
was requested, even if we are going to perform crash recovery first.
Reported by Kyotaro HORIGUCHI. Backpatch to 9.2, like the crash-then-archive
recovery mode.
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index e3d5e10..cdbe305 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -6187,7 +6187,7 @@ CheckRequiredParameterValues(void)
* For archive recovery, the WAL must be generated with at least 'archive'
* wal_level.
*/
- if (InArchiveRecovery ControlFile-wal_level == WAL_LEVEL_MINIMAL)
+ if (ArchiveRecoveryRequested ControlFile-wal_level == WAL_LEVEL_MINIMAL)
{
ereport(WARNING,
(errmsg(WAL was generated with wal_level=minimal, data may be missing),
@@ -6198,7 +6198,7 @@ CheckRequiredParameterValues(void)
* For Hot Standby, the WAL must be generated with 'hot_standby' mode, and
* we must have at least as many backend slots as the primary.
*/
- if (InArchiveRecovery EnableHotStandby)
+ if (ArchiveRecoveryRequested EnableHotStandby)
{
if (ControlFile-wal_level WAL_LEVEL_HOT_STANDBY)
ereport(ERROR,
--
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] jsonb and nested hstore
On Wed, Mar 5, 2014 at 8:39 AM, Bruce Momjian br...@momjian.us wrote: So, I am going to ask a back-track question and ask why we can't move hstore into core. This is exactly the opposite of what should be happening. Now, jsonb might make it into core because of the json precedent but the entire purpose of the extension system is stop dumping everything in the public namespace. Stuff 'in core' becomes locked in stone, forever, because of backwards compatibility concerns, which are IMNSHO, a bigger set of issues than even pg_upgrade related issues. Have we gone through all the new hstore functions and made sure they don't break existing applications? Putting things in core welds your only escape hatch shut. *All* non-sql standard types ought to be in extensions in an ideal world. merlin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: Fwd: [HACKERS] patch: make_timestamp function
Pavel Stehule escribió: Hi I hope, so this patch fix it wtf? -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, 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] jsonb and nested hstore
Andrew Dunstan and...@dunslane.net writes: On 03/05/2014 09:39 AM, Bruce Momjian wrote: So, I am going to ask a back-track question and ask why we can't move hstore into core. Is this a problem with the oids of the hstore data type and functions? Is this a pg_upgrade-only problem? Can this be fixed? Yes, pg_upgrade is the problem, and no, I can't see how it can be fixed. Builtin types have Oids in a certain range. Non-builtin types have Oids outside that range. If you have a clever way to get over that I'd be all ears, but it seems to me insurmountable right now. More to the point: 1. Built-in types have predetermined, fixed OIDs. Types made by extensions do not, and almost certainly will have different OIDs in different existing databases. 2. There's no easy way to change the OID of an existing type during pg_upgrade, because it may be on-disk in (at least) array headers. We could possibly get around #2, if we could think of a secure way for array_out and sibling functions to identify the array type without use of the embedded OID value. I don't know how we could do that though, particularly in polymorphic-function contexts. Also, there might be other cases besides arrays where we've embedded type OIDs in on-disk data; anyone remember? 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] jsonb and nested hstore
On Wed, Mar 5, 2014 at 9:24 AM, Tom Lane t...@sss.pgh.pa.us wrote: Andrew Dunstan and...@dunslane.net writes: On 03/05/2014 09:39 AM, Bruce Momjian wrote: So, I am going to ask a back-track question and ask why we can't move hstore into core. Is this a problem with the oids of the hstore data type and functions? Is this a pg_upgrade-only problem? Can this be fixed? Yes, pg_upgrade is the problem, and no, I can't see how it can be fixed. Builtin types have Oids in a certain range. Non-builtin types have Oids outside that range. If you have a clever way to get over that I'd be all ears, but it seems to me insurmountable right now. More to the point: 1. Built-in types have predetermined, fixed OIDs. Types made by extensions do not, and almost certainly will have different OIDs in different existing databases. 2. There's no easy way to change the OID of an existing type during pg_upgrade, because it may be on-disk in (at least) array headers. We could possibly get around #2, if we could think of a secure way for array_out and sibling functions to identify the array type without use of the embedded OID value. I don't know how we could do that though, particularly in polymorphic-function contexts. Also, there might be other cases besides arrays where we've embedded type OIDs in on-disk data; anyone remember? composite types. merlin -- 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] jsonb and nested hstore
On 03/05/2014 10:24 AM, Tom Lane wrote: Also, there might be other cases besides arrays where we've embedded type OIDs in on-disk data; anyone remember? Don't we do that in composites too? cheers andrew -- 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] jsonb and nested hstore
Merlin Moncure mmonc...@gmail.com writes: On Wed, Mar 5, 2014 at 9:24 AM, Tom Lane t...@sss.pgh.pa.us wrote: Also, there might be other cases besides arrays where we've embedded type OIDs in on-disk data; anyone remember? composite types. But that's only the composite type's own OID, no? So it's not really a problem unless the type we wanted to move into (or out of) core was itself composite. 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] jsonb and nested hstore
On 03/05/2014 10:30 AM, Tom Lane wrote: Merlin Moncure mmonc...@gmail.com writes: On Wed, Mar 5, 2014 at 9:24 AM, Tom Lane t...@sss.pgh.pa.us wrote: Also, there might be other cases besides arrays where we've embedded type OIDs in on-disk data; anyone remember? composite types. But that's only the composite type's own OID, no? So it's not really a problem unless the type we wanted to move into (or out of) core was itself composite. Sure, although that's not entirely impossible to imagine. I admit it seems less likely, and I could accept it as a restriction if we conquered the general case. cheers andrew -- 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] jsonb and nested hstore
On Wed, Mar 5, 2014 at 09:19:33AM -0600, Merlin Moncure wrote: On Wed, Mar 5, 2014 at 8:39 AM, Bruce Momjian br...@momjian.us wrote: So, I am going to ask a back-track question and ask why we can't move hstore into core. This is exactly the opposite of what should be happening. Now, jsonb might make it into core because of the json precedent but the entire purpose of the extension system is stop dumping everything in the public namespace. Stuff 'in core' becomes locked in stone, forever, because of backwards compatibility concerns, which are IMNSHO, a bigger set of issues than even pg_upgrade related issues. Have we gone through all the new hstore functions and made sure they don't break existing applications? Putting things in core welds your only escape hatch shut. *All* non-sql standard types ought to be in extensions in an ideal world. I have seen your opinion on this but there have been enough counter-arguments that I am not ready to head in that direction. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + -- 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] jsonb and nested hstore
On Wed, Mar 5, 2014 at 10:19 AM, Merlin Moncure mmonc...@gmail.com wrote: On Wed, Mar 5, 2014 at 8:39 AM, Bruce Momjian br...@momjian.us wrote: So, I am going to ask a back-track question and ask why we can't move hstore into core. This is exactly the opposite of what should be happening. Now, jsonb might make it into core because of the json precedent but the entire purpose of the extension system is stop dumping everything in the public namespace. Stuff 'in core' becomes locked in stone, forever, because of backwards compatibility concerns, which are IMNSHO, a bigger set of issues than even pg_upgrade related issues. Have we gone through all the new hstore functions and made sure they don't break existing applications? Putting things in core welds your only escape hatch shut. I agree. What concerns me about jsonb is that it doesn't seem very done. If we commit it to core and find out later that we've made some mistakes we'd like to fix, it's going to be difficult and controversial. If it goes on PGXN and turns out to have some problems, then the people responsible for that extension can decide whether and how to preserve backward compatibility, or somebody else can write something completely different. On a theoretical level, I'd absolutely rather have jsonb in core - not because it's in any way theoretically necessary, but because JSON is popular and better support for it will be good for PostgreSQL. But on a practical level I'd rather not ship it in 9.4 than ship something we might later regret. And despite the assertions from various people here that these decisions were all made a long time ago and it's way too late to question them, I don't buy it. There's not a single email on this mailing list clearly laying out the design that we've ended up with, and I'm willing to wager any reasonable amount of money that if someone had posted an email saying hey, we're thinking about setting things up so that jsonb and hstore have the same binary format, but you can't index jsonb directly, you have to cast it to hstore, is everyone OK with that? someone would have written back and said no, that sounds nuts. The reason why input on that particular aspect of the design was not forthcoming isn't because everyone was OK with it; it's because it was never clearly spelled out. Maybe someone will say that this was discussed at last year's PGCon unconference, but surely everyone here knows that a discussion at an unconference 8 months ago doesn't substitute for a discussion on-list. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL 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_tmp files for dropped databases
Tomas Vondra wrote: On 22.2.2014 01:13, Thom Brown wrote: I've noticed that files for dropped databases aren't removed from pg_stat_tmp. After a cluster-wide VACUUM ANALYSE, and restarting Postgres, all the old database pg_stat_tmp files remain. Yeah, that's a bug in pgstat_recv_dropdb() - instead of building path to the temporary file (in pg_stat_tmp), it builds a path to the permanent file in pg_stat. Pushed, thanks. -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, 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] jsonb and nested hstore
Robert Haas robertmh...@gmail.com writes: And despite the assertions from various people here that these decisions were all made a long time ago and it's way too late to question them, I don't buy it. There's not a single email on this mailing list clearly laying out the design that we've ended up with, and I'm willing to wager any reasonable amount of money that if someone had posted an email saying hey, we're thinking about setting things up so that jsonb and hstore have the same binary format, but you can't index jsonb directly, you have to cast it to hstore, is everyone OK with that? someone would have written back and said no, that sounds nuts. The reason why input on that particular aspect of the design was not forthcoming isn't because everyone was OK with it; it's because it was never clearly spelled out. No, that was never the design (I trust). It's where we are today because time ran out to complete jsonb for 9.4, and tossing the index opclasses overboard was one of the last-minute compromises in order to have something submittable. I think it would be a completely defensible decision to postpone jsonb to 9.5 on the grounds that it's not done enough. Now, Josh has laid out arguments why we want jsonb in 9.4 even if it's incomplete. But ISTM that those are fundamentally marketing arguments; on a purely technical basis I think the decision would be to postpone. So it comes down to how you weight marketing vs technical issues, which is something that everyone is likely to see a little bit differently :-( 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] jsonb and nested hstore
On Wed, Mar 5, 2014 at 10:39:56AM -0500, Andrew Dunstan wrote: On 03/05/2014 10:30 AM, Tom Lane wrote: Merlin Moncure mmonc...@gmail.com writes: On Wed, Mar 5, 2014 at 9:24 AM, Tom Lane t...@sss.pgh.pa.us wrote: Also, there might be other cases besides arrays where we've embedded type OIDs in on-disk data; anyone remember? composite types. But that's only the composite type's own OID, no? So it's not really a problem unless the type we wanted to move into (or out of) core was itself composite. Sure, although that's not entirely impossible to imagine. I admit it seems less likely, and I could accept it as a restriction if we conquered the general case. OK, so let's look at the general case. Here is what pg_upgrade preserves: * We control all assignments of pg_class.oid (and relfilenode) so toast * oids are the same between old and new clusters. This is important * because toast oids are stored as toast pointers in user tables. * * While pg_class.oid and pg_class.relfilenode are initially the same * in a cluster, they can diverge due to CLUSTER, REINDEX, or VACUUM * FULL. In the new cluster, pg_class.oid and pg_class.relfilenode will * be the same and will match the old pg_class.oid value. Because of * this, old/new pg_class.relfilenode values will not match if CLUSTER, * REINDEX, or VACUUM FULL have been performed in the old cluster. * * We control all assignments of pg_type.oid because these oids are stored * in user composite type values. * * We control all assignments of pg_enum.oid because these oids are stored * in user tables as enum values. * * We control all assignments of pg_authid.oid because these oids are stored * in pg_largeobject_metadata. It seems only pg_type.oid is an issue for hstore. We can easily modify pg_dump --binary-upgrade mode to suppress the creation of the hstore extension. That should allow user hstore columns to automatically map to the new constant hstore oid. We can also modify pg_upgrade to scan all the user tables for any use of hstore arrays and perhaps composite types and tell the user they have to drop and upgrade those table separately. Again, I am not asking what can be done for 9.4 but what is our final goal, though the pg_upgrade change are minimal as we have done such adjustments in the past. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + -- 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] jsonb and nested hstore
On Wed, Mar 5, 2014 at 9:52 AM, Bruce Momjian br...@momjian.us wrote: On Wed, Mar 5, 2014 at 09:19:33AM -0600, Merlin Moncure wrote: On Wed, Mar 5, 2014 at 8:39 AM, Bruce Momjian br...@momjian.us wrote: So, I am going to ask a back-track question and ask why we can't move hstore into core. This is exactly the opposite of what should be happening. Now, jsonb might make it into core because of the json precedent but the entire purpose of the extension system is stop dumping everything in the public namespace. Stuff 'in core' becomes locked in stone, forever, because of backwards compatibility concerns, which are IMNSHO, a bigger set of issues than even pg_upgrade related issues. Have we gone through all the new hstore functions and made sure they don't break existing applications? Putting things in core welds your only escape hatch shut. *All* non-sql standard types ought to be in extensions in an ideal world. I have seen your opinion on this but there have been enough counter-arguments that I am not ready to head in that direction. The only counter argument given is that this will prevent people from being able to use extensions because they A: can't or won't install contrib packages or B: are too stupid or lazy to type 'create extension json'. Note I'm discussing 'in core extension vs in core built in'. 'out of core extension' loosely translates to 'can't be used by the vast majority of systems. Most corporate IT departments (including mine) have a policy of only installing packages through the operating system packaging to simplify management of deploying updates. Really strict companies might not even allow anything but packages supplied by a vendor like redhat (which in practice keeps you some versions back from the latest). Now, if some crappy hosting company blocks contrib I don't believe at all that this should drive our project management decisions. Postgresql is both a database and increasingly a development language platform. Most good stacks have a system (cpan, npm, etgc) to manage the scope of the installed runtime and it's a routine and expected exercise to leverage that system. merlin -- 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] jsonb and nested hstore
On Mon, Mar 3, 2014 at 11:20 PM, Peter Geoghegan p...@heroku.com wrote: On Mon, Mar 3, 2014 at 6:59 PM, Josh Berkus j...@agliodbs.com wrote: Also, please recognize that the current implementation was what we collectively decided on three months ago, and what Andrew worked pretty hard to implement based on that collective decision. So if we're going to change course, we need a specific reason to change course, not just it seems like a better idea now or I wasn't paying attention then. I'm pretty sure it doesn't work like that. But if it does, what exactly am I insisting on that is inconsistent with that consensus? In what way are we changing course? I think I'm being eminently flexible. I don't want a jsonb type that is broken, as for example by not having a default B-Tree operator class. Why don't you let me get on with it? An excellent question. This thread has become mostly about whether someone (like, say, me, or in this case Peter) is attempting to pull the rug out from under a previously-agreed consensus path forward. But despite my asking, nobody's been able to provide a pointer to any previous discussion of the points under debate. That's because the points that are *actually* being debated here were not discussed previously. I recognize that Josh and Andrew would like to make that the fault of the people who are now raising objections, but it doesn't work like that. The fact that people were and are *generally* in favor of jsonb and hstore doesn't mean they have to like the way that the patches have turned out. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL 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] jsonb and nested hstore
Bruce Momjian br...@momjian.us writes: It seems only pg_type.oid is an issue for hstore. We can easily modify pg_dump --binary-upgrade mode to suppress the creation of the hstore extension. That should allow user hstore columns to automatically map to the new constant hstore oid. We can also modify pg_upgrade to scan all the user tables for any use of hstore arrays and perhaps composite types and tell the user they have to drop and upgrade those table separately. Yeah, and that doesn't seem terribly acceptable. Unless you think the field usage of hstore[] is nil; which maybe it is, I'm not sure what the usage patterns are like. In general it would not be acceptable at all to not be able to support migrations of array columns. 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] jsonb and nested hstore
On 2014-03-05 10:10:23 -0600, Merlin Moncure wrote: On Wed, Mar 5, 2014 at 9:52 AM, Bruce Momjian br...@momjian.us wrote: On Wed, Mar 5, 2014 at 09:19:33AM -0600, Merlin Moncure wrote: *All* non-sql standard types ought to be in extensions in an ideal world. I have seen your opinion on this but there have been enough counter-arguments that I am not ready to head in that direction. The only counter argument given is that this will prevent people from being able to use extensions because they A: can't or won't install contrib packages or B: are too stupid or lazy to type 'create extension json'. Note I'm discussing 'in core extension vs in core built in'. 'out of core extension' loosely translates to 'can't be used by the vast majority of systems. There's the absolutely significant issue that you cannot reasonably write extensions that interact on a C level. You can't call from extension to extension directly, but you can from extension to pg core provided ones. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, 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] jsonb and nested hstore
Merlin Moncure mmonc...@gmail.com writes: *All* non-sql standard types ought to be in extensions in an ideal world. While there's certainly much to be said for the idea that jsonb should be an extension, I don't think we have the technology to package it as a *separate* extension; it'd have to be included in the hstore extension. Which is weird, and quite a mixed message from the marketing standpoint. If I understand Josh's vision of the future, he's expecting that hstore will gradually die off in favor of jsonb, so we don't really want to present the latter as the ugly stepchild. Just out of curiosity, exactly what features are missing from jsonb today that are available with hstore? How long would it take to copy-and-paste all that code, if someone were to decide to do the work instead of argue about it? 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] jsonb and nested hstore
On Wed, Mar 5, 2014 at 10:19 AM, Andres Freund and...@2ndquadrant.com wrote: There's the absolutely significant issue that you cannot reasonably write extensions that interact on a C level. You can't call from extension to extension directly, but you can from extension to pg core provided ones. Certainly. Note I never said that the internal .so can't be in core that both extensions interface with and perhaps wrap. It would be nice to have a intra-extension call system worked out but that in no way plays to the bigger issues at stake. This is all about management of the public API; take a good skeptical look at the history of types like xml, json, geo, money and others. merlin -- 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] jsonb and nested hstore
* Merlin Moncure (mmonc...@gmail.com) wrote: *All* non-sql standard types ought to be in extensions in an ideal world. While I appreciate that you'd like to see it that way, others don't agree (I certainly don't), and that ship sailed quite a long time ago regardless. I'm not advocating putting everything into core, but I agreed with having json in core and further feel jsonb should be there also. I'm not against having hstore either- and I *wish* we'd put ip4r in and replace our existing inet types with it. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] walsender doesn't send keepalives when writes are pending
On 02/25/2014 06:41 PM, Robert Haas wrote:
On Tue, Feb 25, 2014 at 11:23 AM, Andres Freund and...@2ndquadrant.com wrote:
Usually that state will be reached very quickly because before
that we're writing data to the network as fast as it can be read from
disk.
I'm unimpressed. Even if that is in practice true, making the code
self-consistent is a goal of non-trivial value. The timing of sending
keep-alives has no business depending on the state of the write queue,
and right now it doesn't. Your patch would make it depend on that,
mostly by accident AFAICS.
Yeah, the timeout should should be checked regardless of the status of
the write queue. Per the attached patch.
While looking at this, I noticed that the time we sleep between each
iteration is calculated in a funny way. It's not this patch's fault, but
moving the code made it more glaring. After the patch, it looks like this:
TimestampTz timeout;
longsleeptime = 1; /* 10 s */
...
/*
* If wal_sender_timeout is active, sleep in smaller increments
* to not go over the timeout too much. XXX: Why not just sleep
* until the timeout has elapsed?
*/
if (wal_sender_timeout 0)
sleeptime = 1 + (wal_sender_timeout / 10);
/* Sleep until something happens or we time out */
...
WaitLatchOrSocket(MyWalSnd-latch, wakeEvents,
MyProcPort-sock, sleeptime);
...
/*
* Check for replication timeout. Note we ignore the corner case
* possibility that the client replied just as we reached the
* timeout ... he's supposed to reply *before* that.
*/
timeout = TimestampTzPlusMilliseconds(last_reply_timestamp,
wal_sender_timeout);
if (wal_sender_timeout 0 GetCurrentTimestamp() = timeout)
{
...
}
The logic was the same before the patch, but I added the XXX comment
above. Why do we sleep in increments of 1/10 of wal_sender_timeout?
Originally, the code calculated when the next wakeup should happen, by
adding wal_sender_timeout (or replication_timeout, as it was called back
then) to the time of the last reply. Why don't we do that?
The code seems to have just slowly devolved into that. It was changed
from sleep-until-timeout to sleep 1/10th of wal_sender_timeout by a
patch that made walsender send a keep-alive message to the client every
time it wakes up, and I guess the idea was to send a message at an
interval that's 1/10th of the timeout. But that idea is long gone by
now; first, commit da4efa13d801ccc179f1d2c24d8a60c4a2f8ede9 turned off
sending the keep-alive messages by default, and then
6f60fdd7015b032bf49273c99f80913d57eac284 turned them back on, but so
that it's only sent once when 1/2 of wal_sender_timeout has elapsed.
I think that should be changed back to sleep until the next deadline of
when something needs to happen, instead of polling. But that can be a
separate patch.
- Heikki
diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c
index 4fcf3d4..5c7456d 100644
--- a/src/backend/replication/walsender.c
+++ b/src/backend/replication/walsender.c
@@ -1259,6 +1259,27 @@ WalSndLoop(void)
}
/*
+ * If half of wal_sender_timeout has lapsed without receiving any
+ * reply from standby, send a keep-alive message requesting an
+ * immediate reply.
+ */
+ if (wal_sender_timeout 0 !ping_sent)
+ {
+ TimestampTz timeout;
+
+ timeout = TimestampTzPlusMilliseconds(last_reply_timestamp,
+ wal_sender_timeout / 2);
+ if (GetCurrentTimestamp() = timeout)
+ {
+WalSndKeepalive(true);
+ping_sent = true;
+/* Try to flush pending output to the client */
+if (pq_flush_if_writable() != 0)
+ goto send_failure;
+ }
+ }
+
+ /*
* We don't block if not caught up, unless there is unsent data
* pending in which case we'd better block until the socket is
* write-ready. This test is only needed for the case where XLogSend
@@ -1267,7 +1288,7 @@ WalSndLoop(void)
*/
if ((caughtup !streamingDoneSending) || pq_is_send_pending())
{
- TimestampTz timeout = 0;
+ TimestampTz timeout;
long sleeptime = 1; /* 10 s */
int wakeEvents;
@@ -1276,32 +1297,14 @@ WalSndLoop(void)
if (pq_is_send_pending())
wakeEvents |= WL_SOCKET_WRITEABLE;
- else if (wal_sender_timeout 0 !ping_sent)
- {
-/*
- * If half of wal_sender_timeout has lapsed without receiving
- * any reply from standby, send a keep-alive message to
- * standby requesting an immediate reply.
- */
-timeout = TimestampTzPlusMilliseconds(last_reply_timestamp,
- wal_sender_timeout / 2);
-if (GetCurrentTimestamp() = timeout)
-{
- WalSndKeepalive(true);
- ping_sent = true;
- /* Try to flush pending output to the client */
- if
Re: [HACKERS] jsonb and nested hstore
On Wed, Mar 5, 2014 at 11:07 AM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: And despite the assertions from various people here that these decisions were all made a long time ago and it's way too late to question them, I don't buy it. There's not a single email on this mailing list clearly laying out the design that we've ended up with, and I'm willing to wager any reasonable amount of money that if someone had posted an email saying hey, we're thinking about setting things up so that jsonb and hstore have the same binary format, but you can't index jsonb directly, you have to cast it to hstore, is everyone OK with that? someone would have written back and said no, that sounds nuts. The reason why input on that particular aspect of the design was not forthcoming isn't because everyone was OK with it; it's because it was never clearly spelled out. No, that was never the design (I trust). It's where we are today because time ran out to complete jsonb for 9.4, and tossing the index opclasses overboard was one of the last-minute compromises in order to have something submittable. Well, what I was told when I started objecting to the current state of affairs is that it was too late to change course now, which seemed to me to imply that this was the idea all along. On the other hand, Josh also said that there was a plan in the works to ship the missing opclasses on PGXN before 9.4 hits shelves, which is more along the lines of what you're saying. So, hey, I don't know. I think it would be a completely defensible decision to postpone jsonb to 9.5 on the grounds that it's not done enough. Now, Josh has laid out arguments why we want jsonb in 9.4 even if it's incomplete. But ISTM that those are fundamentally marketing arguments; on a purely technical basis I think the decision would be to postpone. So it comes down to how you weight marketing vs technical issues, which is something that everyone is likely to see a little bit differently :-( I don't have any problem shipping incremental progress on important features, but once we ship things that are visible at the SQL level they get awfully hard to change, and my confidence that we won't want to change this is not very high right now. To the extent that we have a jsonb that is missing some features we will eventually want to have, I don't care; that's incremental development at its finest. To the extent that we have a jsonb that makes choices about what to store on disk or expose at the SQL level that we may regret later, that's not incremental development; that's just not being done. Anyone who thinks that digging ourselves out of a backward-compatibility hole will be painless enough to justify the marketing value of the feature has most probably not had to do it. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL 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] jsonb and nested hstore
On Wed, Mar 5, 2014 at 11:24 AM, Tom Lane t...@sss.pgh.pa.us wrote: Merlin Moncure mmonc...@gmail.com writes: *All* non-sql standard types ought to be in extensions in an ideal world. While there's certainly much to be said for the idea that jsonb should be an extension, I don't think we have the technology to package it as a *separate* extension; it'd have to be included in the hstore extension. Which is weird, and quite a mixed message from the marketing standpoint. If I understand Josh's vision of the future, he's expecting that hstore will gradually die off in favor of jsonb, so we don't really want to present the latter as the ugly stepchild. Just out of curiosity, exactly what features are missing from jsonb today that are available with hstore? How long would it take to copy-and-paste all that code, if someone were to decide to do the work instead of argue about it? I believe the main thing is the opclasses. My information might be incomplete. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL 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] jsonb and nested hstore
* Tom Lane (t...@sss.pgh.pa.us) wrote: Just out of curiosity, exactly what features are missing from jsonb today that are available with hstore? How long would it take to copy-and-paste all that code, if someone were to decide to do the work instead of argue about it? Somewhere upthread, Peter seemed to estimate it at a day, if I understood correctly. If that's accurate, I'm certainly behind getting it done and in and moving on. I'm sure no one particularly likes a bunch of copy/pasteing of code, but if it would get us to the point of having a really working jsonb that everyone is happy with, I'm all for it. It's not clear how much different it would be if we waited til 9.5 either- do we anticipate a lot of code changes beyond the copy/paste for these? Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] jsonb and nested hstore
On Wed, Mar 5, 2014 at 10:24 AM, Tom Lane t...@sss.pgh.pa.us wrote: Merlin Moncure mmonc...@gmail.com writes: *All* non-sql standard types ought to be in extensions in an ideal world. While there's certainly much to be said for the idea that jsonb should be an extension, I don't think we have the technology to package it as a *separate* extension; it'd have to be included in the hstore extension. I disagree with that. The shared C bits can live inside the core system and the SQL level hooks and extension specific behaviors could live in an extension. AFAICT moving jsonb to extension is mostly a function of migrating the hard coded SQL hooks out to an extension (I'm probably oversimplifying though). Just out of curiosity, exactly what features are missing from jsonb today that are available with hstore? How long would it take to copy-and-paste all that code, if someone were to decide to do the work instead of argue about it? Basically opclasses, operators (particularly search operators) and functions/operators to manipulate the hstore in place. Personally I'm not inclined to copy/paste the code. I'd also like to see this stuff committed, and don't want to hold up the patch for that unless it's determined for other reasons (and by other people) this is the only reasonable path for 9.4. merlin -- 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] jsonb and nested hstore
* Merlin Moncure (mmonc...@gmail.com) wrote: On Wed, Mar 5, 2014 at 10:24 AM, Tom Lane t...@sss.pgh.pa.us wrote: Merlin Moncure mmonc...@gmail.com writes: *All* non-sql standard types ought to be in extensions in an ideal world. While there's certainly much to be said for the idea that jsonb should be an extension, I don't think we have the technology to package it as a *separate* extension; it'd have to be included in the hstore extension. I disagree with that. The shared C bits can live inside the core system and the SQL level hooks and extension specific behaviors could live in an extension. AFAICT moving jsonb to extension is mostly a function of migrating the hard coded SQL hooks out to an extension (I'm probably oversimplifying though). Yeah, from what I gather you're suggesting, that's more-or-less move it all to core, except that all of the actual interface bits end up in an extension that has to be installed to use what would have to already be there. I don't see that as any kind of improvement. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] jsonb and nested hstore
On Wed, Mar 5, 2014 at 11:16:01AM -0500, Tom Lane wrote: Bruce Momjian br...@momjian.us writes: It seems only pg_type.oid is an issue for hstore. We can easily modify pg_dump --binary-upgrade mode to suppress the creation of the hstore extension. That should allow user hstore columns to automatically map to the new constant hstore oid. We can also modify pg_upgrade to scan all the user tables for any use of hstore arrays and perhaps composite types and tell the user they have to drop and upgrade those table separately. Yeah, and that doesn't seem terribly acceptable. Unless you think the field usage of hstore[] is nil; which maybe it is, I'm not sure what the usage patterns are like. In general it would not be acceptable at all to not be able to support migrations of array columns. It would prevent migration of _hstore_ array columns, which might be acceptable. If we say pg_upgrade can never decline an upgrade, we basically limit changes and increase the odds of needing a total pg_upgrade-breaking release someday to re-adjust everything. I basically think that a split between contrib and core for the internally same data type just isn't sustainable. Another conern is that it doesn't seem we are sure if we want JSONB in core or contrib, at least based on some comments, so we should probably decide that now, as I don't think the decision is going to be any easier in the future. And as discussed above, moving something from contrib to core has its own complexities. I think we also have to break out how much of the feeling that JSONB is not ready is because of problems with the core/contrib split, and how much of it is because of the type itself. I am suggesting that core/contrib split problems are not symptomatic of data type problems, and if address/address the core/contrib split issue, the data type might be just fine. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + -- 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] jsonb and nested hstore
On 03/05/2014 11:34 AM, Stephen Frost wrote: * Tom Lane (t...@sss.pgh.pa.us) wrote: Just out of curiosity, exactly what features are missing from jsonb today that are available with hstore? How long would it take to copy-and-paste all that code, if someone were to decide to do the work instead of argue about it? Somewhere upthread, Peter seemed to estimate it at a day, if I understood correctly. If that's accurate, I'm certainly behind getting it done and in and moving on. I'm sure no one particularly likes a bunch of copy/pasteing of code, but if it would get us to the point of having a really working jsonb that everyone is happy with, I'm all for it. It's not clear how much different it would be if we waited til 9.5 either- do we anticipate a lot of code changes beyond the copy/paste for these? I think that was my estimate, but Peter did offer to do it. He certainly asserted that the effort required would not be great. I'm all for taking up his offer. Incidentally, this would probably have been done quite weeks ago if people had not objected to my doing any more on the feature. Of course missing the GIN/GIST ops was not part of the design. Quite the contrary. cheers andrew -- 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] jsonb and nested hstore
On Wed, Mar 5, 2014 at 11:34:10AM -0500, Stephen Frost wrote: * Tom Lane (t...@sss.pgh.pa.us) wrote: Just out of curiosity, exactly what features are missing from jsonb today that are available with hstore? How long would it take to copy-and-paste all that code, if someone were to decide to do the work instead of argue about it? Somewhere upthread, Peter seemed to estimate it at a day, if I understood correctly. If that's accurate, I'm certainly behind getting it done and in and moving on. I'm sure no one particularly likes a bunch of copy/pasteing of code, but if it would get us to the point of having a really working jsonb that everyone is happy with, I'm all for it. It's not clear how much different it would be if we waited til 9.5 either- do we anticipate a lot of code changes beyond the copy/paste for these? What _would_ be interesting is to move all the hstore code into core, and have hstore contrib just call the hstore core parts. That way, you have one copy of the code, it is shared with JSONB, but hstore remains as an extension that you can change or remove later. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + -- 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] jsonb and nested hstore
On Wed, Mar 5, 2014 at 11:11:51AM -0500, Robert Haas wrote: An excellent question. This thread has become mostly about whether someone (like, say, me, or in this case Peter) is attempting to pull the rug out from under a previously-agreed consensus path forward. But despite my asking, nobody's been able to provide a pointer to any previous discussion of the points under debate. That's because the points that are *actually* being debated here were not discussed previously. I recognize that Josh and Andrew would like to make that the fault of the people who are now raising objections, but it doesn't work like that. The fact that people were and are *generally* in favor of jsonb and hstore doesn't mean they have to like the way that the patches have turned out. I am assuming much of this was discussed verbally, and many of us were not present. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + -- 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] jsonb and nested hstore
On 03/05/2014 11:44 AM, Bruce Momjian wrote: On Wed, Mar 5, 2014 at 11:16:01AM -0500, Tom Lane wrote: Bruce Momjian br...@momjian.us writes: It seems only pg_type.oid is an issue for hstore. We can easily modify pg_dump --binary-upgrade mode to suppress the creation of the hstore extension. That should allow user hstore columns to automatically map to the new constant hstore oid. We can also modify pg_upgrade to scan all the user tables for any use of hstore arrays and perhaps composite types and tell the user they have to drop and upgrade those table separately. Yeah, and that doesn't seem terribly acceptable. Unless you think the field usage of hstore[] is nil; which maybe it is, I'm not sure what the usage patterns are like. In general it would not be acceptable at all to not be able to support migrations of array columns. It would prevent migration of _hstore_ array columns, which might be acceptable. If we say pg_upgrade can never decline an upgrade, we basically limit changes and increase the odds of needing a total pg_upgrade-breaking release someday to re-adjust everything. I basically think that a split between contrib and core for the internally same data type just isn't sustainable. Another conern is that it doesn't seem we are sure if we want JSONB in core or contrib, at least based on some comments, so we should probably decide that now, as I don't think the decision is going to be any easier in the future. And as discussed above, moving something from contrib to core has its own complexities. I think we also have to break out how much of the feeling that JSONB is not ready is because of problems with the core/contrib split, and how much of it is because of the type itself. I am suggesting that core/contrib split problems are not symptomatic of data type problems, and if address/address the core/contrib split issue, the data type might be just fine. Splitting out jsonb to an extension is going to be moderately painful. The json and jsonb functions share some code that's not exposed (and probably shouldn't be). It's not likely to be less painful than implementing the hstore GIN/GIST ops for jsonb, I suspect the reverse. cheers andrew -- 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] jsonb and nested hstore
On Wed, Mar 5, 2014 at 10:43 AM, Stephen Frost sfr...@snowman.net wrote: * Merlin Moncure (mmonc...@gmail.com) wrote: On Wed, Mar 5, 2014 at 10:24 AM, Tom Lane t...@sss.pgh.pa.us wrote: Merlin Moncure mmonc...@gmail.com writes: *All* non-sql standard types ought to be in extensions in an ideal world. While there's certainly much to be said for the idea that jsonb should be an extension, I don't think we have the technology to package it as a *separate* extension; it'd have to be included in the hstore extension. I disagree with that. The shared C bits can live inside the core system and the SQL level hooks and extension specific behaviors could live in an extension. AFAICT moving jsonb to extension is mostly a function of migrating the hard coded SQL hooks out to an extension (I'm probably oversimplifying though). Yeah, from what I gather you're suggesting, that's more-or-less move it all to core, except that all of the actual interface bits end up in an extension that has to be installed to use what would have to already be there. I don't see that as any kind of improvement. If you don't then you simply have not been paying attention to the endless backwards compatibility problems we've faced which are highly ameliorated in an extension heavy world. Also, you're ignoring the fact that having an endlessly accreting set of symbols in the public namespace is not free. Internal C libraries don't have to be supported and don't have any signficant user facing costs by simply being there. merlin -- 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] jsonb and nested hstore
On Wed, Mar 5, 2014 at 11:53:31AM -0500, Andrew Dunstan wrote: I think we also have to break out how much of the feeling that JSONB is not ready is because of problems with the core/contrib split, and how much of it is because of the type itself. I am suggesting that core/contrib split problems are not symptomatic of data type problems, and if address/address the core/contrib split issue, the data type might be just fine. Splitting out jsonb to an extension is going to be moderately painful. The json and jsonb functions share some code that's not exposed (and probably shouldn't be). It's not likely to be less painful than implementing the hstore GIN/GIST ops for jsonb, I suspect the reverse. OK, that's good information. So we have JSONB which ties to a core type, JSON, _and_ to a contrib module, hstore. No wonder it is so complex. I am warming up to the idea of moving hstore internals into core, sharing that with JSONB, and having contrib/hstore just call the core functions when defining its data type. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + -- 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] jsonb and nested hstore
On Mar 5, 2014, at 8:49 AM, Andrew Dunstan and...@dunslane.net wrote: I think that was my estimate, but Peter did offer to do it. He certainly asserted that the effort required would not be great. I'm all for taking up his offer. +1 to this. Can you and Peter collaborate somehow to get it knocked out? Incidentally, this would probably have been done quite weeks ago if people had not objected to my doing any more on the feature. Of course missing the GIN/GIST ops was not part of the design. Quite the contrary. That was my understanding, as well. Best, David -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] parallel pg_dump causes assertion failure in HEAD
$ pg_dump -F d -j 4 -f foo regression pg_dump: [archiver (db)] query failed: pg_dump: [parallel archiver] query was: SET TRANSACTION SNAPSHOT '2130-1' pg_dump: [archiver (db)] query failed: pg_dump: [archiver (db)] query failed: pg_dump: [archiver (db)] query failed: $ postmaster log shows: TRAP: FailedAssertion(!(HistoricSnapshotActive()), File: snapmgr.c, Line: 355) TRAP: FailedAssertion(!(HistoricSnapshotActive()), File: snapmgr.c, Line: 355) TRAP: FailedAssertion(!(HistoricSnapshotActive()), File: snapmgr.c, Line: 355) TRAP: FailedAssertion(!(HistoricSnapshotActive()), File: snapmgr.c, Line: 355) LOG: server process (PID 15069) was terminated by signal 6: Aborted DETAIL: Failed process was running: SET TRANSACTION SNAPSHOT '2130-1' LOG: terminating any other active server processes That Assert appears to have been introduced by commit b89e1510. 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] parallel pg_dump causes assertion failure in HEAD
On 2014-03-05 12:07:43 -0500, Tom Lane wrote: $ pg_dump -F d -j 4 -f foo regression pg_dump: [archiver (db)] query failed: pg_dump: [parallel archiver] query was: SET TRANSACTION SNAPSHOT '2130-1' pg_dump: [archiver (db)] query failed: pg_dump: [archiver (db)] query failed: pg_dump: [archiver (db)] query failed: $ postmaster log shows: TRAP: FailedAssertion(!(HistoricSnapshotActive()), File: snapmgr.c, Line: 355) TRAP: FailedAssertion(!(HistoricSnapshotActive()), File: snapmgr.c, Line: 355) TRAP: FailedAssertion(!(HistoricSnapshotActive()), File: snapmgr.c, Line: 355) TRAP: FailedAssertion(!(HistoricSnapshotActive()), File: snapmgr.c, Line: 355) LOG: server process (PID 15069) was terminated by signal 6: Aborted DETAIL: Failed process was running: SET TRANSACTION SNAPSHOT '2130-1' LOG: terminating any other active server processes That Assert appears to have been introduced by commit b89e1510. Will have a look in an hour or two. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, 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] jsonb and nested hstore
Merlin Moncure mmonc...@gmail.com writes: On Wed, Mar 5, 2014 at 10:24 AM, Tom Lane t...@sss.pgh.pa.us wrote: While there's certainly much to be said for the idea that jsonb should be an extension, I don't think we have the technology to package it as a *separate* extension; it'd have to be included in the hstore extension. I disagree with that. The shared C bits can live inside the core system and the SQL level hooks and extension specific behaviors could live in an extension. That approach abandons every bit of value in an extension, no? You certainly don't get to fix bugs outside a core-system release cycle. 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] jsonb and nested hstore
On Wed, Mar 5, 2014 at 11:50 AM, Bruce Momjian br...@momjian.us wrote: On Wed, Mar 5, 2014 at 11:34:10AM -0500, Stephen Frost wrote: * Tom Lane (t...@sss.pgh.pa.us) wrote: Just out of curiosity, exactly what features are missing from jsonb today that are available with hstore? How long would it take to copy-and-paste all that code, if someone were to decide to do the work instead of argue about it? Somewhere upthread, Peter seemed to estimate it at a day, if I understood correctly. If that's accurate, I'm certainly behind getting it done and in and moving on. I'm sure no one particularly likes a bunch of copy/pasteing of code, but if it would get us to the point of having a really working jsonb that everyone is happy with, I'm all for it. It's not clear how much different it would be if we waited til 9.5 either- do we anticipate a lot of code changes beyond the copy/paste for these? What _would_ be interesting is to move all the hstore code into core, and have hstore contrib just call the hstore core parts. That way, you have one copy of the code, it is shared with JSONB, but hstore remains as an extension that you can change or remove later. That seems like an approach possibly worth investigating. It's not too different from what we did when we moved text search into core. The basic idea seems to be that we want jsonb in core, and we expect it to replace hstore, but we can't get just get rid of hstore because it has too many users. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL 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] [PATCH] Negative Transition Aggregate Functions (WIP)
On 5 March 2014 14:35, Florian Pflug f...@phlo.org wrote: On Mar4, 2014, at 21:09 , Dean Rasheed dean.a.rash...@gmail.com wrote: On 3 March 2014 23:00, Florian Pflug f...@phlo.org wrote: * In show_windowagg_info(), this calculation looks suspicious to me: double tperrow = winaggstate-aggfwdtrans / (inst-nloops * inst-ntuples); If the node is executed multiple times, aggfwdtrans will be reset in each loop, so the transitions per row figure will be under-estimated. ISTM that if you want to report on this, you'd need aggfwdtrans to be reset once per query, but I'm not sure exactly how to do that. ... Actually, I think it's misleading to only count forward transition function calls, because a call to the inverse transition function still represents a state transition, and is likely to be around the same cost. For a window of size 2, there would not be much advantage to using inverse transition functions, because it would be around 2 transitions per row either way. True. In fact, I pondered whether to avoid using the inverse transition function for windows of 2 rows. In the end, I didn't because I felt that it makes custom aggregates harder to test. On the question of whether to count inverse transition function calls - the idea of the EXPLAIN VERBOSE ANALYZE output isn't really to show the number of state transitions, but rather to show whether the aggregation has O(n) or O(n^2) behaviour. The idea being that a value close to 1 means inverse transition function works as expected, and larger values mean not working so well. Regarding multiple evaluations - I think I based the behaviour on how ntuples works, which also only reports the value of the last evaluation I think. But maybe I'm confused about this. No, it doesn't look like that's correct for multiple loops. Consider this example: ... It turns out that show_windowagg_info() is only called once at the end, with ntuples=100, nloops=4 and aggfwdtrans=100, so it's computing tperrow=100/(4*100)=0.25, which then gets truncated to 0.2. So to get 1, you'd have to use this formula: double tperrow = winaggstate-aggfwdtrans / inst-ntuples; Hm, so I *was* confused - seems I mixed up ntuples (which counts the total number of tuples over all loops) with what we report as rows (i.e. the average number of rows per loop). Thanks for clearing that up! I'm still not convinced that's the most useful thing to report though. Personally, I'd prefer to just see the separate counts, e.g.: - WindowAgg (cost=0.00..22.50 rows=1000 width=4) (actual time=0.019..0.094 rows=25 loops=4) Output: sum(i.i) OVER (?) Forward transitions: 25 Inverse transitions: 25 IMO that gives a clearer picture of what's going on. My problem with this is that if there are multiple aggregates, the numbers don't really count the number of forward and reverse transfer function invocations. What nodeWindowAgg.c really counts is the number of *rows* it has to fetch from the tuple store to perform forward aggregations - the relevant code snippet is if (numaggs_restart 0) winstate-aggfwdtrans += (winstate-aggregatedupto - winstate-frameheadpos); else winstate-aggfwdtrans += (winstate-aggregatedupto - aggregatedupto_nonrestarted); Now we could of course report these figures per aggregate instead of only once per aggregation node. But as I said earlier, my guess is that people usually won't be interested in the precise counts - most likely, what you want to know is how much do the restarts cost me The problem I have with the single Transitions Per Row figure, and the idea that a value close to 1.0 is supposed to be good, is that it's not really true. For example, with a window size of 2 and a perfect invertible aggregate, you'd get a value of 1.0, but with a non-invertible aggregate you'd get a value of 2.0, when actually it wouldn't do any better if it had an inverse. I think trying to represent this as a single number is too simplistic. When I added the EXPLAIN stuff, I initially simply reported the number of times nodeWindowAgg has to restart the aggregation. The problem with that approach is that not all restarts carry the same cost. If the frames either don't overlap at all or are identical, restarts don't cause any additional work. And for fixed-length frames (BETWEEN n PRECEEDING AND m FOLLOWING), the performance effects of restarts depends on m-n. Which is why I made it count the number of aggregated input rows instead. Having said that, I' not really 100% happy with the name Transitions Per Row for this - it was simply the best I could come up with that was reasonably short. And I'm certainly open to reporting the absolute count instead of a factor relative to ntuples. If we made it an absolute count, would calling this Aggregated Rows work for you? I'm not sure about naming, but I think my
Re: [HACKERS] jsonb and nested hstore
* Robert Haas (robertmh...@gmail.com) wrote: On Wed, Mar 5, 2014 at 11:50 AM, Bruce Momjian br...@momjian.us wrote: What _would_ be interesting is to move all the hstore code into core, and have hstore contrib just call the hstore core parts. That way, you have one copy of the code, it is shared with JSONB, but hstore remains as an extension that you can change or remove later. That seems like an approach possibly worth investigating. It's not too different from what we did when we moved text search into core. The basic idea seems to be that we want jsonb in core, and we expect it to replace hstore, but we can't get just get rid of hstore because it has too many users. This might be valuable for hstore, specifically, because we can't easily move it into core. I'm fine with that- the disagreement I have is with the more general idea that everything not-defined-by-committee should be in shim extensions which just provide basically the catalog entries for types which are otherwise all in core. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] jsonb and nested hstore
On Wed, Mar 5, 2014 at 12:26:13PM -0500, Robert Haas wrote: It's not clear how much different it would be if we waited til 9.5 either- do we anticipate a lot of code changes beyond the copy/paste for these? What _would_ be interesting is to move all the hstore code into core, and have hstore contrib just call the hstore core parts. That way, you have one copy of the code, it is shared with JSONB, but hstore remains as an extension that you can change or remove later. That seems like an approach possibly worth investigating. It's not too different from what we did when we moved text search into core. The basic idea seems to be that we want jsonb in core, and we expect it to replace hstore, but we can't get just get rid of hstore because it has too many users. Yes. It eliminates the problem of code duplication, but keeps hstore in contrib for flexibility and compatibility. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + -- 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] Negative Transition Aggregate Functions (WIP)
Dean Rasheed dean.a.rash...@gmail.com writes: I think we really need a larger consensus on this though, so I'd be interested to hear what others think. My advice is to lose the EXPLAIN output entirely. If the authors of the patch can't agree on what it means, what hope have everyday users got of making sense of it? 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] jsonb and nested hstore
On 03/05/2014 12:01 PM, Bruce Momjian wrote: On Wed, Mar 5, 2014 at 11:53:31AM -0500, Andrew Dunstan wrote: I think we also have to break out how much of the feeling that JSONB is not ready is because of problems with the core/contrib split, and how much of it is because of the type itself. I am suggesting that core/contrib split problems are not symptomatic of data type problems, and if address/address the core/contrib split issue, the data type might be just fine. Splitting out jsonb to an extension is going to be moderately painful. The json and jsonb functions share some code that's not exposed (and probably shouldn't be). It's not likely to be less painful than implementing the hstore GIN/GIST ops for jsonb, I suspect the reverse. OK, that's good information. So we have JSONB which ties to a core type, JSON, _and_ to a contrib module, hstore. No wonder it is so complex. Well, ties to is a loose term. It's hstore in these patches that depends on jsonb - necessarily since we can't have core code depend on an extension. I am warming up to the idea of moving hstore internals into core, sharing that with JSONB, and having contrib/hstore just call the core functions when defining its data type. Right, at least the parts they need in common. That's how I'd handle the GIN/GIST ops, for example. cheers andrew -- 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] parallel pg_dump causes assertion failure in HEAD
On March 5, 2014 6:07:43 PM CET, Tom Lane t...@sss.pgh.pa.us wrote: $ pg_dump -F d -j 4 -f foo regression pg_dump: [archiver (db)] query failed: pg_dump: [parallel archiver] query was: SET TRANSACTION SNAPSHOT '2130-1' pg_dump: [archiver (db)] query failed: pg_dump: [archiver (db)] query failed: pg_dump: [archiver (db)] query failed: $ postmaster log shows: TRAP: FailedAssertion(!(HistoricSnapshotActive()), File: snapmgr.c, Line: 355) TRAP: FailedAssertion(!(HistoricSnapshotActive()), File: snapmgr.c, Line: 355) TRAP: FailedAssertion(!(HistoricSnapshotActive()), File: snapmgr.c, Line: 355) TRAP: FailedAssertion(!(HistoricSnapshotActive()), File: snapmgr.c, Line: 355) LOG: server process (PID 15069) was terminated by signal 6: Aborted DETAIL: Failed process was running: SET TRANSACTION SNAPSHOT '2130-1' LOG: terminating any other active server processes That Assert appears to have been introduced by commit b89e1510. It's a typo. It should make sure there is *no* historical snapshot. Will later test if it really works, but I'd be surprised if not. Andres -- Please excuse brevity and formatting - I am writing this on my mobile phone. Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, 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] jsonb and nested hstore
* Merlin Moncure (mmonc...@gmail.com) wrote: On Wed, Mar 5, 2014 at 10:43 AM, Stephen Frost sfr...@snowman.net wrote: Yeah, from what I gather you're suggesting, that's more-or-less move it all to core, except that all of the actual interface bits end up in an extension that has to be installed to use what would have to already be there. I don't see that as any kind of improvement. If you don't then you simply have not been paying attention to the endless backwards compatibility problems we've faced which are highly ameliorated in an extension heavy world. We have backwards compatibility problems because we don't want to *break* things for people. Moving things into extensions doesn't magically fix that- if you break something in a backwards-incompatible way then you're going to cause a lot of grief for people. Doing that to everyone who uses hstore, just because it's an extension, doesn't make it acceptable. On this thread we're already argueing about exactly that issue and how to avoid breaking things for those users were we to move hstore into core. Also, you're ignoring the fact that having an endlessly accreting set of symbols in the public namespace is not free. Internal C libraries don't have to be supported and don't have any signficant user facing costs by simply being there. I *really* hate how extensions end up getting dumped into the public schema and I'm not a big fan for having huge search_paths either. As I mentioned earlier- I'm also not advocating that everything be put into core. I don't follow what you mean by Internal C libraries don't have to be supported because, clearly, anything released would have to be supported and if the extension is calling into a C interface then we'd have to support that interface for that extension *and anyone else who uses it*. We don't get to say oh, this C function can only be used by extensions we bless. We already worry less about breaking backwards compatibility for C-level functions across PG major versions, but that's true for both in-core hooks and extensions. Thanks, Stephen signature.asc Description: Digital signature
[HACKERS] Re: API change advice: Passing plan invalidation info from the rewriter into the planner?
Craig Ringer escribió:
One of the remaining issues with row security is how to pass plan
invalidation information generated in the rewriter back into the planner.
I think I already asked this, but would it work to extract this info by
walking the rewritten list of queries instead; and in case it would,
would that be any easier than the API change you're proposing?
We use Query structs throughout the rewriter and planner; it doesn't
make sense to add a List* field for PlanInvalItem nodes and an Oid field
for the user ID to the Query node when it's only ever going to get used
for the top level Query node returned by the rewriter, and only for long
enough to copy the data into PlannerGlobal.
So there is an assumption that you can't have a subquery that uses a
different role ID than the main query. That sounds fine, and anyway I
don't think we're prepared to deal with differing userids for
subqueries, so the proposal that it belongs only on the top-level node
is acceptable. And from there, it seems that not putting the info in
Query (which would be a waste everywhere else than the toplevel query
node) is sensible.
The alternative seems to be changing the return type of QueryRewrite,
introducing a new node type, say:
struct RewriteResult {
Query*productQuery;
Oid planUserId;
List* planInvalItems;
}
This seems cleaner, and more extensible, but it means changing a fair
bit of API, including:
pg_plan_query
planner
standard_planner
planner_hook_type
QueryRewrite
I think we should just bite the bullet and do the change (a new struct,
I assume, not a new node). It will cause an incompatibility to anyone
that has written planner hooks, but probably the number of such hooks is
not very large anyway.
I don't think we should base decisions on the amount of backpatching
pain we cause, for patches that involve new functionality such as this
one. We commit patches that will cause future merge conflicts all the
time.
I'm inclined to bite the bullet and make the API change. It'll be a
pain, but I can see future uses for passing global info out of the
rewriter rather than shoving it into per-Query structures. I'd define a
RewriteResult and pass that down into all the rewriter internal
functions, then return the outer query wrapped in it.
Is there already something in Query that could be a toplevel struct
member only?
--
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, 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] jsonb and nested hstore
On Wed, Mar 5, 2014 at 11:19 AM, Tom Lane t...@sss.pgh.pa.us wrote: Merlin Moncure mmonc...@gmail.com writes: On Wed, Mar 5, 2014 at 10:24 AM, Tom Lane t...@sss.pgh.pa.us wrote: While there's certainly much to be said for the idea that jsonb should be an extension, I don't think we have the technology to package it as a *separate* extension; it'd have to be included in the hstore extension. I disagree with that. The shared C bits can live inside the core system and the SQL level hooks and extension specific behaviors could live in an extension. That approach abandons every bit of value in an extension, no? You certainly don't get to fix bugs outside a core-system release cycle. That's core vs non core debate. Just about everyone (including me) wants json and hstore to live in core -- meaning packaged, shipped, supported, and documented with the postgresql source code releases. Only an elite set of broadly useful and popular extensions get that honor of which json is most certainly one. Moreover, you give up nothing except the debate/approval issues to get your code in core. If you want to release off cycle, you can certainly do that and enterprising users can simply install the extension manually (or perhaps via pgxn) instead of via contrib. BTW,This is yet another thing that becomes impossible if you don't extension (on top of legacy/backwards compatibility issues and general bloat which is IMNSHO already a pretty severe situation). merlin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] decoding typos...
Four files with comment typos. --- src/backend/replication/logical/decode.c.orig 2014-03-05 18:44:31.725317514 +0100 +++ src/backend/replication/logical/decode.c 2014-03-05 18:45:09.577190828 +0100 @@ -497,7 +497,7 @@ /* * Check whether we are interested in this specific transaction, and tell - * the the reorderbuffer to forget the content of the (sub-)transactions + * the reorderbuffer to forget the content of the (sub-)transactions * if not. * * There basically two reasons we might not be interested in this--- src/backend/replication/logical/logical.c.orig 2014-03-05 18:44:46.188875504 +0100 +++ src/backend/replication/logical/logical.c 2014-03-05 18:45:49.321041107 +0100 @@ -179,7 +179,7 @@ * perform the use-case dependent, actual, work. * * Needs to be called while in a memory context that's at least as long lived - * as the the decoding context because further memory contexts will be created + * as the decoding context because further memory contexts will be created * inside it. * * Returns an initialized decoding context after calling the output plugin's @@ -334,7 +334,7 @@ * perform the use-case dependent, actual, work. * * Needs to be called while in a memory context that's at least as long lived - * as the the decoding context because further memory contexts will be created + * as the decoding context because further memory contexts will be created * inside it. * * Returns an initialized decoding context after calling the output plugin's--- src/backend/replication/logical/reorderbuffer.c.orig 2014-03-05 18:46:16.515250612 +0100 +++ src/backend/replication/logical/reorderbuffer.c 2014-03-05 18:46:31.943799569 +0100 @@ -2741,7 +2741,7 @@ * * A tuple with a cmin but no cmax (and thus no combocid) got * deleted/updated in another transaction than the one which created it * which we are looking at right now. As only one of cmin, cmax or combocid - * is actually stored in the heap we don't have access to the the value we + * is actually stored in the heap we don't have access to the value we * need anymore. * * To resolve those problems we have a per-transaction hash of (cmin,--- src/backend/storage/ipc/procarray.c.orig 2014-03-05 18:46:43.473461974 +0100 +++ src/backend/storage/ipc/procarray.c 2014-03-05 18:46:55.592106219 +0100 @@ -1948,7 +1948,7 @@ /* * Acquire XidGenLock, so no transactions can acquire an xid while we're * running. If no transaction with xid were running concurrently a new xid - * could influence the the RecentXmin et al. + * could influence the RecentXmin et al. * * We initialize the computation to nextXid since that's guaranteed to be * a safe, albeit pessimal, value. -- 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] decoding typos...
On Wed, Mar 5, 2014 at 12:57 PM, Erik Rijkers e...@xs4all.nl wrote: Four files with comment typos. Committed, thanks. For future reference, a single patch is easier than four separate ones. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL 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] jsonb and nested hstore
On 03/05/2014 09:26 AM, Robert Haas wrote: What _would_ be interesting is to move all the hstore code into core, and have hstore contrib just call the hstore core parts. That way, you have one copy of the code, it is shared with JSONB, but hstore remains as an extension that you can change or remove later. That seems like an approach possibly worth investigating. It's not too different from what we did when we moved text search into core. The basic idea seems to be that we want jsonb in core, and we expect it to replace hstore, but we can't get just get rid of hstore because it has too many users Yes, please! This was the original approach that we talked about and everyone agreed to, and what Andrew has been trying to implement. -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.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] [PATCH] Negative Transition Aggregate Functions (WIP)
Tom, I did not follow the thread very close, so I need to look what the ambiguity is there, however I would love to see window rescans in explain analyze. I have great experience in tuning Oracle queries. There are features in PostgreSQL's explain analyze that I miss badly in Oracle: 'rows removed by filter' is my favourite one. Improving explain analyze is great for performance analysis. I would say target audience for 'explain analyze' is not all the users, but someone closer to 'performance engineers'. Those beings are used to triple-check results and build/validate hypothesis since all the counters tend to lie, so 'a bit misleading counter' is not a showstopper. I did not think of 'non-being-able-to-use-negative-transition-since-floats-do-not-commute' before. Thanks to this discussion I see what kind of dragons live here. I would vote (if I had any vote at all) for the inclusion of performance statistics to explain analyze (e.g. number of rescans and number of rows fed to aggregate) provided performance impact is tolerable in both regular and explain analyze mode. I wonder how Oracle handles negative transition (does it?), however that is a different discussion. Regards, Vladimir Sitnikov
Re: [HACKERS] Changeset Extraction v7.9.1
On Tue, Mar 4, 2014 at 6:26 PM, Andres Freund and...@2ndquadrant.com wrote:
On 2014-03-03 16:48:15 -0500, Robert Haas wrote:
OK, I've committed the 0001 patch, which is the core of this feature,
with a bit of minor additional hacking.
Attached are the rebased patches that are remaining.
Changes:
* minor conflict due to 7558cc95d31edb
* removal of the last XXX in the walsender patch by setting the
timestamps in the 'd' messages correctly.
* Some documentation wordsmithing by Craig
The walsender patch currently contains the changes about feedback we
argued about elsewhere, I guess I either need to back them out, or we
need to argue out that minor bit.
OK, reading through the walsender patch (0002 in this series):
PLEASE stop using a comma to join two independent thoughts. Don't do
it in the comments, and definitely don't do it in error messages. I'm
referring to things like this: invalid value for option
\replication\, legal values are false, 0, true, 1 or database. I
know that you're not a native English speaker, and if you were
submitting a smaller amount of code I wouldn't just fix it for you,
but you do this A LOT and I've probably fixed a hundred instances of
it already and I can't cope with fixing another hundred. In code
comments, a semicolon is often an adequate substitute, but that even
with that change this won't do for an error message. For that, you
should copy the style of something done elsewhere. For example, in
this instance, perhaps look to this precedent:
rhaas=# set synchronous_commit = barfle;
ERROR: invalid value for parameter synchronous_commit: barfle
HINT: Available values: local, remote_write, on, off.
This patch still treats allow a walsender to connect to a database
as a separate feature from allow logical replication. I'm not
convinced that's a good idea. What you're proposing to do is allow
replication=database in addition to replication=true and
replication=false. But how about instead allowing
replication=physical and replication=logical? physical can just be
a synonym for true and the database name can be ignored as it is
today. logical can pay attention the database name. I'm not
totally wedded to that exact design, but basically, I'm not
comfortable with allowing a physical WAL sender to connect to a
database in advance of a concrete need. We might want to leave some
room to go there later if we think it's a likely direction, but
allowing people to do it in advance of any functional advantage just
seems like a recipe for bugs. Practically nobody will run that way so
breakage won't be timely detected. (And no, I don't know exactly what
will break.)
+ if (am_cascading_walsender !RecoveryInProgress())
+ {
+ ereport(LOG,
+ (errmsg(terminating walsender process
to force cascaded standby to update timeline and reconnect)));
+ walsender_ready_to_stop = true;
+ }
Does this apply to logical replication? Seems like it could at least
have a comment.
+ /*
+* XXX: For feedback purposes it would be nicer to set sentPtr to
+* cmd-startpoint, but we use it to know where to read xlog in the main
+* loop...
+*/
I'm not sure I understand this.
WalSndWriteData() looks kind of cut-and-pasty.
WalSndWaitForWal() is yet another slightly-modified copy of the same
darn loop. Surely we need a better way of doing this. It's
absolutely inevitable that some future hacker will not patch every
copy of this loop in some situation where that is required.
There might be more; that's all I see at the moment.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL 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] API change advice: Passing plan invalidation info from the rewriter into the planner?
Craig Ringer cr...@hobby.2ndquadrant.com writes: One of the remaining issues with row security is how to pass plan invalidation information generated in the rewriter back into the planner. With row security, it's necessary to set a field in PlannerGlobal, tracking the user ID of the user the query was planned for if row security was applied. It is also necessary to add a PlanInvalItem for the user ID. TBH I'd just add a user OID field in struct Query and not hack up a bunch of existing function APIs. It's not much worse than the existing constraintDeps field. The PlanInvalItem could perfectly well be generated by the planner, no, if it has the user OID? But I'm not real sure why you need it. I don't see the reason for an invalidation triggered by user ID. What exactly about the *user*, and not something else, would trigger plan invalidation? What we do need is a notion that a plan cache entry might only be valid for a specific calling user ID; but that's a matter for cache entry lookup not for subsequent invalidation. 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] Changeset Extraction v7.9.1
On 03/05/2014 10:49 AM, Robert Haas wrote: This patch still treats allow a walsender to connect to a database as a separate feature from allow logical replication. I'm not convinced that's a good idea. What you're proposing to do is allow replication=database in addition to replication=true and replication=false. But how about instead allowing replication=physical and replication=logical? physical can just be a synonym for true and the database name can be ignored as it is today. logical can pay attention the database name. I'm not totally wedded to that exact design, but basically, I'm not comfortable with allowing a physical WAL sender to connect to a database in advance of a concrete need. We might want to leave some room to go there later if we think it's a likely direction, but allowing people to do it in advance of any functional advantage just seems like a recipe for bugs. Practically nobody will run that way so breakage won't be timely detected. (And no, I don't know exactly what will break.) Personally, I'd prefer to just have the permission here governed by the existing replication permission; why make things complicated for users? But maybe Andres has some other requirement he's trying to fullfill? -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.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] jsonb and nested hstore
On Wed, Mar 5, 2014 at 8:30 AM, Robert Haas robertmh...@gmail.com wrote: Just out of curiosity, exactly what features are missing from jsonb today that are available with hstore? How long would it take to copy-and-paste all that code, if someone were to decide to do the work instead of argue about it? I believe the main thing is the opclasses. Yes, that's right. A large volume of code currently proposed for hstore2 is much less valuable than those operators sufficient to implement the hstore2 opclasses. If you assume that hstore will become a legacy extension that we won't add anything to (including everything proposed in any patch posted to this thread), and jsonb will go in core (which is of course more or less just hstore2 with a few json extras), the amount of code redundantly shared between core and an unchanged hstore turns out to not be that bad. I hope to have a precise answer to just how bad soon. -- Peter Geoghegan -- 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] UNION ALL on partitioned tables won't use indices.
Kyotaro HORIGUCHI horiguchi.kyot...@lab.ntt.co.jp writes: ec_relids has never included child relids. relation.h says that, |Relids ec_relids; /* all relids appearing in ec_members */ ... |Relids em_relids; /* all relids appearing in em_expr */ Ah. Those comments could use improvement, I guess. 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] Changeset Extraction v7.9.1
On Wed, Mar 5, 2014 at 1:57 PM, Josh Berkus j...@agliodbs.com wrote: On 03/05/2014 10:49 AM, Robert Haas wrote: This patch still treats allow a walsender to connect to a database as a separate feature from allow logical replication. I'm not convinced that's a good idea. What you're proposing to do is allow replication=database in addition to replication=true and replication=false. But how about instead allowing replication=physical and replication=logical? physical can just be a synonym for true and the database name can be ignored as it is today. logical can pay attention the database name. I'm not totally wedded to that exact design, but basically, I'm not comfortable with allowing a physical WAL sender to connect to a database in advance of a concrete need. We might want to leave some room to go there later if we think it's a likely direction, but allowing people to do it in advance of any functional advantage just seems like a recipe for bugs. Practically nobody will run that way so breakage won't be timely detected. (And no, I don't know exactly what will break.) Personally, I'd prefer to just have the permission here governed by the existing replication permission; why make things complicated for users? But maybe Andres has some other requirement he's trying to fullfill? This isn't about permissions; it's about the fact that physical replication is cluster-wide, but logical replication is per-database. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL 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] jsonb and nested hstore
On Wed, Mar 5, 2014 at 10:59:37AM -0800, Peter Geoghegan wrote: On Wed, Mar 5, 2014 at 8:30 AM, Robert Haas robertmh...@gmail.com wrote: Just out of curiosity, exactly what features are missing from jsonb today that are available with hstore? How long would it take to copy-and-paste all that code, if someone were to decide to do the work instead of argue about it? I believe the main thing is the opclasses. Yes, that's right. A large volume of code currently proposed for hstore2 is much less valuable than those operators sufficient to implement the hstore2 opclasses. If you assume that hstore will become a legacy extension that we won't add anything to (including everything proposed in any patch posted to this thread), and jsonb will go in core (which is of course more or less just hstore2 with a few json extras), the amount of code redundantly shared between core and an unchanged hstore turns out to not be that bad. I hope to have a precise answer to just how bad soon. Can you clarify what hstore2 is? It that the name of a type? Is that hierarchical hstore with the same hstore name? -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + -- 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] GSoC propostal - CREATE SCHEMA ... LIKE ...
On Tue, Mar 4, 2014 at 3:33 PM, Fabrízio de Royes Mello fabriziome...@gmail.com wrote: Is the TODO item CREATE SCHEMA ... LIKE ... [1] a good GSoC project? Maybe. I'm not sure that everyone would agree that it's a good idea. And it'd probably involve mucking with a bunch of tablecmds.c code that is kind of a mess already. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL 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] jsonb and nested hstore
On 03/05/2014 11:05 AM, Bruce Momjian wrote: Can you clarify what hstore2 is? It that the name of a type? Is that hierarchical hstore with the same hstore name? hstore2 == nested heirarchical hstore. It's just a shorthand; there won't be any actual type called hstore2, by design. Unlike the json users, the hstore users are going to get an automatic upgrade whether they want it or not. Mind you, I can't see a reason NOT to want it ... -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.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] The behavior of CheckRequiredParameterValues()
On Wed, Mar 5, 2014 at 5:13 PM, Amit Langote amitlangot...@gmail.com wrote: On Wed, Mar 5, 2014 at 12:07 PM, Amit Langote amitlangot...@gmail.com wrote: On Wed, Mar 5, 2014 at 2:09 AM, Sawada Masahiko sawada.m...@gmail.com wrote: xlog.c:6177 if (ControlFile-wal_level WAL_LEVEL_HOT_STANDBY) ereport(ERROR, (errmsg(hot standby is not possible because wal_level was not So we have to start and stop standby server with changed wal_level(i.g., hot_standby) if we want to enable hot standby. In this case, I think that the standby server didn't need to confirm wal_level value of ControlFile. I think that it should confirm value which is written in postgreql.conf. I think checking it from the control file on a standby in recovery means that we should confirm if the *wal_level with which the WAL was generated* is sufficient to now become a hot standby after recovery finishes. Sorry, should have said: *become a hot standby after recovery reaches a consistent state Thank you for explain! I understood it! Regards, --- Sawada Masahiko -- 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] [BUGS] BUG #9223: plperlu result memory leak
Alex Hunsaker escribió: On Tue, Feb 25, 2014 at 6:56 AM, Sergey Burladyan eshkin...@gmail.com wrote: It looks like I found the problem, Perl use reference count and something that is called Mortal for memory management. As I understand it, mortal is free after FREETMPS. Plperl call FREETMPS in plperl_call_perl_func() but after it, plperl ask perl interpreter again for new mortal SV variables, for example, in hek2cstr from plperl_sv_to_datum, and this new SV is newer freed. So I think hek2cstr is the only place we leak (its the only place I can see that allocates a mortal sv without being wrapped in ENTER/SAVETMPS/FREETMPS/LEAVE). Does the attached fix it for you? Can I bug you into verifying what supported releases need this patch, and to which does it backpatch cleanly? And if there's any to which it doesn't, can I further bug you into providing one that does? Thanks, -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, 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: Custom Scan APIs (Re: [HACKERS] Custom Plan node)
Kouhei Kaigai kai...@ak.jp.nec.com writes: * Please drop the whole register_custom_provider/get_custom_provider API. One thing I was worrying about is how copyObject() and nodeToString() support set of function pointer tables around custom-scan node, however, you suggested to change the assumption here. So, I think these functions become unnecessary. If we allow the extension to control copyObject behavior, it can do what it likes with the function-struct pointer. I think the typical case would be that it's a simple pointer to a never-copied static constant. But you could imagine that it's a pointer to a struct embedded further down in the custom path or plan node, if the extension wants different functions for different plans. If we had to support stringToNode() for paths or plans, things would get much more complicated, but we don't (and there are already lots of other things that would be difficult for that). The reason why CustomScan is derived from Scan is, some of backend code wants to know rtindex of the relation to be referenced by this CustomScan. The scanrelid of Scan is used in three points: nodeCustom.c, setrefs.c and explain.c. The usage in nodeCustom.c is just for service routines, and the usage in setrefs.c can be moved to the extension according to your suggestion. We need to investigate the usage in explain.c; ExplainPreScanNode() walks around the nodes to collect relids referenced in this query. If we don't want to put a callback for this specific usage, it is a reasonable choice to show the backend the associated scanrelid of CustomScan. I think we have to add another callback for that :-(. It's a pain since it's such a trivial point; but the existing code cannot support a custom node referencing more than one RTE, which seems possible for join or append type cases. Probably, the hunk around show_customscan_info() call can be entirely moved to the extension side. If so, I want ExplainNode() being an extern function, because it allows extensions to print underlying plan-nodes. I haven't looked at what explain.c would have to expose to make this workable, but yeah, we will probably have to export a few things. Are you saying the hard-wired portion in setrefs.c can be moved to the extension side? If fix_scan_expr() become extern function, I think it is feasible. My recollection is that fix_scan_expr() is a bit specialized. Not sure exactly what we'd have to export there --- but we'd have to do it anyway. What you had in the patch was a hook that could be called, but no way for it to do what it would likely need to do. * Get rid of the CUSTOM_VAR business, too (including custom_tupdesc). In case of Var-node that references joined-relations, it shall be replaced to either INNER_VAR or OUTER_VAR according the location of underlying relations. It eventually makes ExecEvalScalarVar() to reference either ecxt_innertuple or ecxt_outertuple, however, it is problematic if we already consolidated tuples come from the both side into one. So? If you did that, then you wouldn't have renumbered the Vars as INNER/OUTER. I don't believe that CUSTOM_VAR is necessary at all; if it is needed, then there would also be a need for an additional tuple slot in executor contexts, which you haven't provided. For example, the enhanced postgres_fdw fetches the result set of remote join query, thus a tuple contains the fields come from both side. In this case, what varno shall be suitable to put? That would be a scan situation, and the vars could reference the scan tuple slot. Which in fact was the implementation you were using, so how is CUSTOM_VAR adding anything? * Why is there more than one call site for add_scan_path_hook? I don't see the need for the calling macro with the randomly inconsistent name, either. Where is the best place to do? Even though I cannot imagine the situation to run sub-query or cte by extensions, its path is constructed during set_rel_size(), so I had to put the hook for each set__pathlist() functions. Hm. We could still call the hook in set_rel_pathlist, if we were to get rid of the individual calls to set_cheapest and do that in one spot at the bottom of set_rel_pathlist (after the hook call). Calling set_cheapest in one place seems more consistent anyway. 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] [BUG FIX] Compare returned value by socket() against PGINVALID_SOCKET instead of 0
Joel Jacobson wrote: Hi, I've tried to fix some bugs reported by Andrey Karpov in an article at http://www.viva64.com/en/b/0227/ The value returned by socket() is unsigned on Windows and can thus not be checked if less than zero to detect an error, instead PGINVALID_SOCKET should be used, which is hard-coded to -1 on non-windows platforms. Can I bug you into verifying what supported releases need this patch, and to which does it backpatch cleanly? And if there's any to which it doesn't, can I further bug you into providing one that does? -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, 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] jsonb and nested hstore
On Wed, Mar 5, 2014 at 10:59:37AM -0800, Peter Geoghegan wrote: On Wed, Mar 5, 2014 at 8:30 AM, Robert Haas robertmh...@gmail.com wrote: Just out of curiosity, exactly what features are missing from jsonb today that are available with hstore? How long would it take to copy-and-paste all that code, if someone were to decide to do the work instead of argue about it? I believe the main thing is the opclasses. Yes, that's right. A large volume of code currently proposed for hstore2 is much less valuable than those operators sufficient to implement the hstore2 opclasses. If you assume that hstore will become a legacy extension that we won't add anything to (including everything proposed in any patch posted to this thread), and jsonb will go in core (which is of course more or less just hstore2 with a few json extras), the amount of code redundantly shared between core and an unchanged hstore turns out to not be that bad. I hope to have a precise answer to just how bad soon. So, now knowing that hstore2 is just hierarchical hstore using the same hstore type name, you are saying that we are keeping the non-hierarchical code in contrib, and the rest goes into core --- that makes sense, and from a code maintenance perspective, I like that the non-hierarchical hstore code is not going in core. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + -- 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] GSoC propostal - CREATE SCHEMA ... LIKE ...
On Wed, Mar 5, 2014 at 4:06 PM, Robert Haas robertmh...@gmail.com wrote: On Tue, Mar 4, 2014 at 3:33 PM, Fabrízio de Royes Mello fabriziome...@gmail.com wrote: Is the TODO item CREATE SCHEMA ... LIKE ... [1] a good GSoC project? Maybe. I'm not sure that everyone would agree that it's a good idea. And it'd probably involve mucking with a bunch of tablecmds.c code that is kind of a mess already. If already exists functions that dump objects maybe this work be easier, or I'm completely wrong? Well, maybe I must concentrate my efforts to make unlogged table logged proposal instead of try to find another one! :-) -- Fabrízio de Royes Mello Consultoria/Coaching PostgreSQL Timbira: http://www.timbira.com.br Blog sobre TI: http://fabriziomello.blogspot.com Perfil Linkedin: http://br.linkedin.com/in/fabriziomello Twitter: http://twitter.com/fabriziomello
Re: [HACKERS] [BUGS] BUG #9223: plperlu result memory leak
On Wed, Mar 5, 2014 at 12:22 PM, Alvaro Herrera alvhe...@2ndquadrant.com wrote: Can I bug you into verifying what supported releases need this patch, and to which does it backpatch cleanly? And if there's any to which it doesn't, can I further bug you into providing one that does? Sure! Not bugging at all. I'll dig into this in a few hours. -- 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] jsonb and nested hstore
On Wed, Mar 5, 2014 at 11:44 AM, Stephen Frost sfr...@snowman.net wrote: * Merlin Moncure (mmonc...@gmail.com) wrote: On Wed, Mar 5, 2014 at 10:43 AM, Stephen Frost sfr...@snowman.net wrote: Yeah, from what I gather you're suggesting, that's more-or-less move it all to core, except that all of the actual interface bits end up in an extension that has to be installed to use what would have to already be there. I don't see that as any kind of improvement. If you don't then you simply have not been paying attention to the endless backwards compatibility problems we've faced which are highly ameliorated in an extension heavy world. We have backwards compatibility problems because we don't want to *break* things for people. Moving things into extensions doesn't magically fix that- if you break something in a backwards-incompatible way then you're going to cause a lot of grief for people. It doesn't magically fix it, but at least provides a way forward. If the function you want to modify is in an extension 'foo', you get to put your new stuff in 'foo2' extension. That way your users do not have to adjust all the code you would have broken. Perhaps for in-core extensions you offer the old one in contrib for a while until a reasonable amount of time passes then move it out to pgxn. This is a vastly better system than the choices we have now, which is A. break code or B. do nothing. Also, you're ignoring the fact that having an endlessly accreting set of symbols in the public namespace is not free. Internal C libraries don't have to be supported and don't have any signficant user facing costs by simply being there. I *really* hate how extensions end up getting dumped into the public schema and I'm not a big fan for having huge search_paths either. At least with extensions you have control over this. mentioned earlier- I'm also not advocating that everything be put into core. I don't follow what you mean by Internal C libraries don't have to be supported because, I mean, we are free to change them or delete them. They do not come with the legacy that user facing API comes. They also do not bloat the public namespace. merlin -- 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] Changeset Extraction v7.9.1
Hi,
On 2014-03-05 13:49:23 -0500, Robert Haas wrote:
PLEASE stop using a comma to join two independent thoughts.
Ok. I'll try.
Is this a personal preference, or a general rule? There seems to be a
fair amount of comments in pg doing so?
This patch still treats allow a walsender to connect to a database
as a separate feature from allow logical replication. I'm not
convinced that's a good idea. What you're proposing to do is allow
replication=database in addition to replication=true and
replication=false. But how about instead allowing
replication=physical and replication=logical? physical can just be
a synonym for true and the database name can be ignored as it is
today. logical can pay attention the database name. I'm not
totally wedded to that exact design, but basically, I'm not
comfortable with allowing a physical WAL sender to connect to a
database in advance of a concrete need. We might want to leave some
room to go there later if we think it's a likely direction, but
allowing people to do it in advance of any functional advantage just
seems like a recipe for bugs. Practically nobody will run that way so
breakage won't be timely detected. (And no, I don't know exactly what
will break.)
I am only mildly against doing so, so you certainly can nudge me in that
direction.
Would you want to refuse using existing commands in logical mode? It's not
unrealistic to first want to perform a basebackup and then establish a
logical slot to replay from there on. It's probably not too bad to force
separate connections there, but it seems like a somewhwat pointless
exercise to me?
+ if (am_cascading_walsender !RecoveryInProgress())
+ {
+ ereport(LOG,
+ (errmsg(terminating walsender process
to force cascaded standby to update timeline and reconnect)));
+ walsender_ready_to_stop = true;
+ }
Does this apply to logical replication? Seems like it could at least
have a comment.
I think it does make sense to force a disconnect in this case to
simplify code, but you're right, both a comment and some TLC for the
message are in order.
WalSndWriteData() looks kind of cut-and-pasty.
You mean from the WalSndLoop? Yea. I tried to reduce it by introducing
WalSndCheckTimeOut() but I think at the very least
WalSndComputeTimeOut() is in order.
I very much dislike having the three different event loops, but it's
pretty much forced by the design of the xlogreader. My xlogreader
version didn't block when it neeeded to wait for WAL but just returned
need input/output, but with the eventually committed version you're
pretty much forced to block inside the read_page callback.
I don't really have a idea how we could sensibly unify them atm.
Greetings,
Andres Freund
--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, 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] GSoC on WAL-logging hash indexes
On Mon, Mar 3, 2014 at 8:12 AM, Robert Haas robertmh...@gmail.com wrote: Unfortunately, I don't believe that it's possible to do this easily today because of the way bucket splits are handled. I wrote about this previously here, with an idea for solving the problem: http://www.postgresql.org/message-id/ca+tgmozymojsrfxhxq06g8jhjxqcskvdihb_8z_7nc7hj7i...@mail.gmail.com Sadly, no one responded. :-( On Mon, Mar 3, 2014 at 9:39 AM, Tan Tran tankimt...@gmail.com wrote: Thanks for alerting me to your previous idea. While I don't know enough about Postgresql internals to judge its merits yet, I'll write some pseudocode based on it in my proposal; and I'll relegate it to a reach proposal alongside a more straightforward one. Tan Hi Tan, I'm not familiar with the inner workings of the GSoC, but I don't know if this can be relegated to a stretch goal. WAL logging is an all or nothing thing. I think that, to be applied to the codebase (which I assume is the goal of GSoC), all actions need to be implemented. That is probably why this has remained open so long: there is no incremental way to get the code written. (But I would like to see it get done, I don't want to discourage that.) Cheers, Jeff
[HACKERS] Disable hot-update functionality
Hi All, Is there any ways by which i can disable the hot-update functionality? -- Regards, Rohit Goyal
Re: [HACKERS] parallel pg_dump causes assertion failure in HEAD
On 2014-03-05 18:39:52 +0100, Andres Freund wrote:
On March 5, 2014 6:07:43 PM CET, Tom Lane t...@sss.pgh.pa.us wrote:
$ pg_dump -F d -j 4 -f foo regression
pg_dump: [archiver (db)] query failed: pg_dump: [parallel archiver]
query was: SET TRANSACTION SNAPSHOT '2130-1'
pg_dump: [archiver (db)] query failed: pg_dump: [archiver (db)] query
failed: pg_dump: [archiver (db)] query failed: $
postmaster log shows:
TRAP: FailedAssertion(!(HistoricSnapshotActive()), File: snapmgr.c,
Line: 355)
TRAP: FailedAssertion(!(HistoricSnapshotActive()), File: snapmgr.c,
Line: 355)
TRAP: FailedAssertion(!(HistoricSnapshotActive()), File: snapmgr.c,
Line: 355)
TRAP: FailedAssertion(!(HistoricSnapshotActive()), File: snapmgr.c,
Line: 355)
LOG: server process (PID 15069) was terminated by signal 6: Aborted
DETAIL: Failed process was running: SET TRANSACTION SNAPSHOT
'2130-1'
LOG: terminating any other active server processes
That Assert appears to have been introduced by commit b89e1510.
It's a typo. It should make sure there is *no* historical
snapshot. Will later test if it really works, but I'd be surprised if
not.
So, after crashing my laptop by doing pg_dump -j16 on a cluster with 4GB
of shared_buffers (causing 16 parallel coredumps to be written) I can
confirm that indeed that the reason is just a typo in an assert.
Patch fixing that attached, it also includes a fix for a comment in a
related checks for historical snapshots.
Greetings,
Andres Freund
--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training Services
From 20da7e70cf5bc3c4dc943e71ec77f53ecc5f785a Mon Sep 17 00:00:00 2001
From: Andres Freund and...@anarazel.de
Date: Wed, 5 Mar 2014 21:20:56 +0100
Subject: [PATCH] Fix typo in Assert() statement causing
SetTransactionSnapshot() to fail.
Also fix comment that hasn't got the message about removing the need
for temporarily suspending historical snapshots.
---
src/backend/utils/time/snapmgr.c | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/src/backend/utils/time/snapmgr.c b/src/backend/utils/time/snapmgr.c
index 4146527..9802fa7 100644
--- a/src/backend/utils/time/snapmgr.c
+++ b/src/backend/utils/time/snapmgr.c
@@ -261,9 +261,11 @@ Snapshot
GetCatalogSnapshot(Oid relid)
{
/*
- * Return historic snapshot if we're doing logical decoding, but
- * return a non-historic, snapshot if we temporarily are doing up2date
- * lookups.
+ * Return historic snapshot while we're doing logical decoding, so we can
+ * see the appropriate state of the catalog.
+ *
+ * This is the primary reason for needing to reset the system caches after
+ * finishing decoding.
*/
if (HistoricSnapshotActive())
return HistoricSnapshot;
@@ -352,7 +354,7 @@ SetTransactionSnapshot(Snapshot sourcesnap, TransactionId sourcexid)
Assert(RegisteredSnapshots == 0);
Assert(FirstXactSnapshot == NULL);
- Assert(HistoricSnapshotActive());
+ Assert(!HistoricSnapshotActive());
/*
* Even though we are not going to use the snapshot it computes, we must
--
1.8.3.251.g1462b67
--
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] Disable hot-update functionality
On Wed, Mar 5, 2014 at 5:32 PM, Rohit Goyal rhtgyl...@gmail.com wrote: Hi All, Is there any ways by which i can disable the hot-update functionality? Why do you want do that? Grettings, -- Fabrízio de Royes Mello Consultoria/Coaching PostgreSQL Timbira: http://www.timbira.com.br Blog sobre TI: http://fabriziomello.blogspot.com Perfil Linkedin: http://br.linkedin.com/in/fabriziomello Twitter: http://twitter.com/fabriziomello
Re: [HACKERS] jsonb and nested hstore
* Merlin Moncure (mmonc...@gmail.com) wrote: On Wed, Mar 5, 2014 at 11:44 AM, Stephen Frost sfr...@snowman.net wrote: We have backwards compatibility problems because we don't want to *break* things for people. Moving things into extensions doesn't magically fix that- if you break something in a backwards-incompatible way then you're going to cause a lot of grief for people. It doesn't magically fix it, but at least provides a way forward. If the function you want to modify is in an extension 'foo', you get to put your new stuff in 'foo2' extension. That way your users do not have to adjust all the code you would have broken. Perhaps for in-core extensions you offer the old one in contrib for a while until a reasonable amount of time passes then move it out to pgxn. This is a vastly better system than the choices we have now, which is A. break code or B. do nothing. I don't see why we can't do exactly what you're suggesting in core. This whole thread is about doing exactly that, in fact, which is why we're talking about 'jsonb' instead of just 'json'. I agree that we don't push too hard to remove things from core, but it's not like we've had a whole ton of success kicking things out of -contrib either. I *really* hate how extensions end up getting dumped into the public schema and I'm not a big fan for having huge search_paths either. At least with extensions you have control over this. Yeah, but I much prefer how things end up in pg_catalog rather than public or individual schemas. mentioned earlier- I'm also not advocating that everything be put into core. I don't follow what you mean by Internal C libraries don't have to be supported because, I mean, we are free to change them or delete them. They do not come with the legacy that user facing API comes. They also do not bloat the public namespace. But we actually *aren't* free to change or delete them- which is what I was getting at. Certainly, in back-branches we regularly worry about breaking things for users of C functions, and there is some consideration for them even in major version changes. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] jsonb and nested hstore
Merlin Moncure escribió: On Wed, Mar 5, 2014 at 11:44 AM, Stephen Frost sfr...@snowman.net wrote: We have backwards compatibility problems because we don't want to *break* things for people. Moving things into extensions doesn't magically fix that- if you break something in a backwards-incompatible way then you're going to cause a lot of grief for people. It doesn't magically fix it, but at least provides a way forward. If the function you want to modify is in an extension 'foo', you get to put your new stuff in 'foo2' extension. That way your users do not have to adjust all the code you would have broken. Perhaps for in-core extensions you offer the old one in contrib for a while until a reasonable amount of time passes then move it out to pgxn. Uhm. Would it work to define a new version of foo, say 2.0, but keep the old 1.2 version the default? That way, if you want to keep the old foo you do nothing (after both fresh install and pg_upgrade), and if you want to upgrade to the new code, it's just an ALTER EXTENSION UPDATE away. -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, 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] walsender doesn't send keepalives when writes are pending
On 2014-03-05 18:26:13 +0200, Heikki Linnakangas wrote: On 02/25/2014 06:41 PM, Robert Haas wrote: On Tue, Feb 25, 2014 at 11:23 AM, Andres Freund and...@2ndquadrant.com wrote: Usually that state will be reached very quickly because before that we're writing data to the network as fast as it can be read from disk. I'm unimpressed. Even if that is in practice true, making the code self-consistent is a goal of non-trivial value. The timing of sending keep-alives has no business depending on the state of the write queue, and right now it doesn't. Your patch would make it depend on that, mostly by accident AFAICS. I still don't see how my proposed patch increases the dependency, rather the contrary, it's less dependant on the flushing behaviour. But I agree that a more sweeping change is a good idea. The logic was the same before the patch, but I added the XXX comment above. Why do we sleep in increments of 1/10 of wal_sender_timeout? Originally, the code calculated when the next wakeup should happen, by adding wal_sender_timeout (or replication_timeout, as it was called back then) to the time of the last reply. Why don't we do that? [ archeology ] It imo makes sense to wakeup after last_reply + wal_sender_timeout/2, so a requested reply actually has time to arrive, but otherwise I agree. I think your patch makes sense. Additionally imo the timeout checking should be moved outside the if (caughtup || pq_is_send_pending()), but that's probably a separate patch. Any chance you could apply your patch soon? I've a patch pending that'll surely conflict with this and it seems better to fix it first. Greetings, Andres Freund -- 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] Changeset Extraction v7.9.1
On Wed, Mar 5, 2014 at 3:04 PM, Andres Freund and...@2ndquadrant.com wrote: Hi, On 2014-03-05 13:49:23 -0500, Robert Haas wrote: PLEASE stop using a comma to join two independent thoughts. Ok. I'll try. Is this a personal preference, or a general rule? There seems to be a fair amount of comments in pg doing so? http://en.wikipedia.org/wiki/Comma_splice This patch still treats allow a walsender to connect to a database as a separate feature from allow logical replication. I'm not convinced that's a good idea. What you're proposing to do is allow replication=database in addition to replication=true and replication=false. But how about instead allowing replication=physical and replication=logical? physical can just be a synonym for true and the database name can be ignored as it is today. logical can pay attention the database name. I'm not totally wedded to that exact design, but basically, I'm not comfortable with allowing a physical WAL sender to connect to a database in advance of a concrete need. We might want to leave some room to go there later if we think it's a likely direction, but allowing people to do it in advance of any functional advantage just seems like a recipe for bugs. Practically nobody will run that way so breakage won't be timely detected. (And no, I don't know exactly what will break.) I am only mildly against doing so, so you certainly can nudge me in that direction. Would you want to refuse using existing commands in logical mode? It's not unrealistic to first want to perform a basebackup and then establish a logical slot to replay from there on. It's probably not too bad to force separate connections there, but it seems like a somewhwat pointless exercise to me? Hmm, that's an interesting point. I didn't consider the case of a base backup followed by replication, on the same connection. That might be sufficient justification for doing it the way you have it. WalSndWriteData() looks kind of cut-and-pasty. You mean from the WalSndLoop? Yea. I tried to reduce it by introducing WalSndCheckTimeOut() but I think at the very least WalSndComputeTimeOut() is in order. I very much dislike having the three different event loops, but it's pretty much forced by the design of the xlogreader. My xlogreader version didn't block when it neeeded to wait for WAL but just returned need input/output, but with the eventually committed version you're pretty much forced to block inside the read_page callback. I don't really have a idea how we could sensibly unify them atm. WalSndLoop(void (*gutsfn)())? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL 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] jsonb and nested hstore
On Wed, Mar 5, 2014 at 2:45 PM, Alvaro Herrera alvhe...@2ndquadrant.com wrote:
Merlin Moncure escribió:
It doesn't magically fix it, but at least provides a way forward. If
the function you want to modify is in an extension 'foo', you get to
put your new stuff in 'foo2' extension. That way your users do not
have to adjust all the code you would have broken. Perhaps for
in-core extensions you offer the old one in contrib for a while until
a reasonable amount of time passes then move it out to pgxn.
Uhm. Would it work to define a new version of foo, say 2.0, but keep
the old 1.2 version the default? That way, if you want to keep the old
foo you do nothing (after both fresh install and pg_upgrade), and if you
want to upgrade to the new code, it's just an ALTER EXTENSION UPDATE
away.
Certainly. The important point though is that neither option is
available if the old stuff is locked into the public namespace.
Consider various warts like the array ('array_upper' et al) API or geo
types. We're stuck with them. Even with jsonb: it may be the hot new
thing *today* but 5 years down the line there's json2 that does all
kinds of wonderful things we haven't thought about -- what if it
displaces current usages? The very same people who are arguing that
jsonb should not be in an extension are the ones arguing json is
legacy and to be superseded. These two points of view IMO are
directly in conflict: if json would have been an extension than the
path to deprecation is clear. Now the json functions are in the
public namespace. Forever (or at least for a very long time).
On Wed, Mar 5, 2014 at 2:46 PM, Stephen Frost sfr...@snowman.net wrote:
I don't see why we can't do exactly what you're suggesting in core.
Because you can't (if you're defining core to mean 'not an
extension'). Functions can't be removed or changed because of legacy
application support. In an extension world, they can -- albeit not
'magically', but at least it can be done.
merlin
--
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] Disable hot-update functionality
On Wed, Mar 5, 2014 at 12:32 PM, Rohit Goyal rhtgyl...@gmail.com wrote: Hi All, Is there any ways by which i can disable the hot-update functionality? Build an index on a volatile column. For example, to force pgbench to bypass HOT updates for testing purposes I build an index on pgbench_accounts (abalance). Cheers, Jeff
Re: [HACKERS] [PATCH] Negative Transition Aggregate Functions (WIP)
On Mar5, 2014, at 18:37 , Tom Lane t...@sss.pgh.pa.us wrote: Dean Rasheed dean.a.rash...@gmail.com writes: I think we really need a larger consensus on this though, so I'd be interested to hear what others think. My advice is to lose the EXPLAIN output entirely. If the authors of the patch can't agree on what it means, what hope have everyday users got of making sense of it? The question isn't what the current output means, but whether it's a good metric to report or not. If we don't report anything, then how would a user check whether a query is slow because of O(n^2) behaviour of a windowed aggregate, or because of some other reasons? If inevitability where a purely static property, then maybe we could get away with that, and say check whether your aggregates are invertible or not. But since we have partially invertible aggregates, the performance characteristics depends on the input data, so we IMHO need some way for users to check what's actually happening. best regards, Florian Pflug -- 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] Changeset Extraction v7.9.1
On 2014-03-05 17:05:24 -0500, Robert Haas wrote: I very much dislike having the three different event loops, but it's pretty much forced by the design of the xlogreader. My xlogreader version didn't block when it neeeded to wait for WAL but just returned need input/output, but with the eventually committed version you're pretty much forced to block inside the read_page callback. I don't really have a idea how we could sensibly unify them atm. WalSndLoop(void (*gutsfn)())? The problem is that they are actually different. In the WalSndLoop we're also maintaining the walsender's state, in WalSndWriteData() we're just waiting for writes to be flushed, in WalSndWaitForWal we're primarily waiting for the flush pointer to pass some LSN. And the timing of the individual checks isn't trivial (just added some more comments about it). I'll simplify it by pulling out more common code, maybe it'll become apparent how it should look. Greetings, Andres Freund PS: I so far considered my language counted poetic, that's why I used the splicing comma so liberally... Thanks for the link. -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, 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] jsonb and nested hstore
* Merlin Moncure (mmonc...@gmail.com) wrote: On Wed, Mar 5, 2014 at 2:46 PM, Stephen Frost sfr...@snowman.net wrote: I don't see why we can't do exactly what you're suggesting in core. Because you can't (if you're defining core to mean 'not an extension'). Functions can't be removed or changed because of legacy application support. In an extension world, they can -- albeit not 'magically', but at least it can be done. That simply isn't accurate on either level- if there is concern about application support, that can apply equally to core and contrib, and we certainly *can* remove and/or redefine functions in core with sufficient cause. It's just not something we do lightly for things living in either core or contrib. For an example, consider the FDW API, particularly what we did between 9.1 and 9.2. Thanks, Stephen signature.asc Description: Digital signature
[HACKERS] Unportable coding in reorderbuffer.h
I don't believe that this is legal per C90:
typedef struct ReorderBufferChange
{
XLogRecPtrlsn;
/* type of change */
union
{
enum ReorderBufferChangeType action;
/* do not leak internal enum values to the outside */
intaction_internal;
};
...
That union field needs a name. Yeah, I realize this makes references
to the contained fields more verbose. Tough. Our project standard
is we compile on C90 compilers, and we're not moving that goalpost
just to save you a couple of keystrokes.
Worse, this isn't portable even if you assume a C99 compiler. There is
nothing at all constraining the compiler to make the enum field the same
size as the int field, and if they're not, reading the int will not yield
the same value you wrote as an enum (or vice versa). It might've
accidentally failed to fail on the compilers you tested on, but it's
still wrong.
The other nameless union in ReorderBufferChange needs a name too.
By the time you get done fixing the portability issue, I suspect you
won't have a union at all for the first case. I'm not real sure that
you need a union for the second case either --- is it really important
to shave a few bytes from the size of this struct? So you don't
necessarily need to do a notation change for the second union.
What drew my attention to it was an older gcc version complaining
thusly:
In file included from ../../../../src/include/replication/decode.h:13,
from decode.c:38:
../../../../src/include/replication/reorderbuffer.h:60: warning: unnamed
struct/union that defines no instances
../../../../src/include/replication/reorderbuffer.h:94: warning: unnamed
struct/union that defines no instances
decode.c: In function `DecodeInsert':
decode.c:588: structure has no member named `action'
decode.c:589: structure has no member named `tp'
decode.c:595: structure has no member named `tp'
decode.c:599: structure has no member named `tp'
decode.c: In function `DecodeUpdate':
decode.c:628: structure has no member named `action'
decode.c:629: structure has no member named `tp'
decode.c:637: structure has no member named `tp'
decode.c:641: structure has no member named `tp'
decode.c:651: structure has no member named `tp'
decode.c:654: structure has no member named `tp'
... etc etc ...
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
