Re: [HACKERS] patch (for 9.1) string functions
2010/7/13 Pavel Stehule :
> so this is actualised patch:
> * concat_sql removed
> * left, right, reverse and concat are in core
> * printf and concat_ws are in contrib
> * format show "" as NULL string
> * removed an using of wide chars
I think function codes in the core (concat, format, left, right,
and reverse) are ready for committers. They also have docs, but
the names are not listed in Index page (bookindex.html).
Please add
funcname
in func.sgml for each new function.
However, I have a couple of comments to stringfunc module. sprintf()
and concat_ws() are not installed by default, but provided by the module.
> todo:
> NULL handling for printf function
I like for null arguments. It is just same as format() and RAISE.
=== Questions ===
* concat_ws() transforms NULLs into empty strings.
Is it an intended behavior and compatible with MySQL?
Note that string_agg() doesn't add separators to NULLs.
=# SELECT coalesce(concat_ws(',', 'A', NULL, 'B'), '(null)');
coalesce
--
A,,B
(1 row)
* concat_ws() returns NULL when the separator is NULL.
Is it an intended behavior and compatible with MySQL?
=# SELECT coalesce(concat_ws(NULL, 'A', NULL, 'B'), '(null)');
coalesce
--
(null)
(1 row)
=== Trivial issues ===
* Some function prototypes are declared but not used.
We can just remove them.
- mb_string_info()
- stringfunc_concat(PG_FUNCTION_ARGS);
- stringfunc_left(PG_FUNCTION_ARGS);
- stringfunc_right(PG_FUNCTION_ARGS);
- stringfunc_reverse(PG_FUNCTION_ARGS);
* Some error messages need to be improved.
For example, "1th" is wrong.
=# select sprintf('>>>%*s<<<', NULL, 'abcdef');
ERROR: null value not allowed
HINT: width (1th) arguments is NULL
* sprintf() has some typos in error messages
For example, "sprinf".
--
Itagaki Takahiro
--
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] log files and permissions
I think the patch is almost ready for committer except the following three issues: 2010/7/13 Fujii Masao : >> + if (!*value || *endptr || file_mode < 0 || file_mode > 0777) > The sticky bit cannot be set via log_file_mode. Is this intentional? We should also check the value not to be something like 0699. How about checking it with (file_mode & ~0666) != 0 ? >> +#ifndef WIN32 >> + chmod(filename, Log_file_mode); >> +#endif > Don't we need to check the return value of chmod()? I prefer umask() rather than chmod() here. >> +const char *show_log_file_mode(void); > You forgot to write the show_log_file_mode()? I was not able to find that > in the patch. I want show_log_file_mode to print the setting value in octal format. -- Itagaki Takahiro -- 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] dblink regression failure in HEAD
Andrew Dunstan writes: > Tom Lane wrote: >> What this looks like to me is that dblink.c doesn't contain the fix >> that the new regression test is checking for. >> >> pangolin is pulling from the git mirror, which I still don't trust >> further than I can throw it. How about you? > There is something very odd about that machine. It started failing 5 > days ago. Then it stopped, then it started again. A few experiments later: I can reproduce the failure shown on pangolin exactly if I revert the latest changes in sql/dblink.sql and expected/dblink.out, while keeping dblink.c up to date. So I guessed wrong on which file was out of sync, but I say confidently that this is a repository sync problem. 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] dblink regression failure in HEAD
Tom Lane wrote: Itagaki Takahiro writes: I found regression test for dblink in HEAD was failed on my machine. One buildfarm machine also reported the same failure. What this looks like to me is that dblink.c doesn't contain the fix that the new regression test is checking for. pangolin is pulling from the git mirror, which I still don't trust further than I can throw it. How about you? There is something very odd about that machine. It started failing 5 days ago. Then it stopped, then it started again. As for the git mirror, I can only speak for the mirror I maintain, which is validated on every live branch every day against CVS, so far without a hiccup, and has four of my buildfarm members happily building against it (and possibly others). 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] dblink regression failure in HEAD
Excerpts from Tom Lane's message of lun jul 12 23:02:05 -0400 2010: > pangolin is pulling from the git mirror, which I still don't trust > further than I can throw it. How about you? Easy enough to check -- just verify the $PostgreSQL$ tag in the file. Oh wait ... -- 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] dblink regression failure in HEAD
Itagaki Takahiro writes: > I found regression test for dblink in HEAD was failed on my machine. > One buildfarm machine also reported the same failure. What this looks like to me is that dblink.c doesn't contain the fix that the new regression test is checking for. pangolin is pulling from the git mirror, which I still don't trust further than I can throw it. How about you? regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] dblink regression failure in HEAD
I found regression test for dblink in HEAD was failed on my machine.
One buildfarm machine also reported the same failure.
http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=pangolin&dt=2010-07-12%2013:37:06
It seems to come from the recent fix for dropped column support,
but I'm not sure why other machines can pass the test.
= pgsql.29332/contrib/dblink/regression.diffs
===
*** /home/slux/buildfarm/HEAD/pgsql.29332/contrib/dblink/expected/dblink.out
Mon
Jul 12 13:37:07 2010
--- /home/slux/buildfarm/HEAD/pgsql.29332/contrib/dblink/results/dblink.out
Mon
Jul 12 13:51:40 2010
***
*** 905,926
ADD COLUMN col4 INT NOT NULL DEFAULT 42;
SELECT dblink_build_sql_insert('test_dropped', '2', 1,
ARRAY['1'::TEXT], ARRAY['2'::TEXT]);
! dblink_build_sql_insert
! ---
! INSERT INTO test_dropped(id,col2b,col3,col4) VALUES('2','113','foo','42')
! (1 row)
!
SELECT dblink_build_sql_update('test_dropped', '2', 1,
ARRAY['1'::TEXT], ARRAY['2'::TEXT]);
! dblink_build_sql_update
!
---
! UPDATE test_dropped SET id = '2', col2b = '113', col3 = 'foo', col4
= '42' WHERE id = '2'
! (1 row)
!
SELECT dblink_build_sql_delete('test_dropped', '2', 1,
ARRAY['2'::TEXT]);
! dblink_build_sql_delete
! -
! DELETE FROM test_dropped WHERE id = '2'
(1 row)
--- 905,918
ADD COLUMN col4 INT NOT NULL DEFAULT 42;
SELECT dblink_build_sql_insert('test_dropped', '2', 1,
ARRAY['1'::TEXT], ARRAY['2'::TEXT]);
! ERROR: source row not found
SELECT dblink_build_sql_update('test_dropped', '2', 1,
ARRAY['1'::TEXT], ARRAY['2'::TEXT]);
! ERROR: source row not found
SELECT dblink_build_sql_delete('test_dropped', '2', 1,
ARRAY['2'::TEXT]);
! dblink_build_sql_delete
!
! DELETE FROM test_dropped WHERE col2b = '2'
(1 row)
--
Itagaki Takahiro
--
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] log files and permissions
On Mon, Jul 12, 2010 at 7:36 PM, Martin Pihlak wrote:
> Itagaki Takahiro wrote:
>> I checked "log_file_mode GUC" patch, and found a couple of Windows-specific
>> and translation issues.
>
> Thank you for the review. Attached patch attempts to fix these issues.
> + if (!*value || *endptr || file_mode < 0 || file_mode > 0777)
> + {
> + ereport(GUC_complaint_elevel(source),
> + (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
> + errmsg("invalid value for parameter
> \"log_file_mode\"")));
The sticky bit cannot be set via log_file_mode. Is this intentional?
> +#ifndef WIN32
> + chmod(filename, Log_file_mode);
> +#endif
Don't we need to check the return value of chmod()?
> +const char *assign_log_file_mode(const char *value,
> + bool doit, GucSource
> source);
> +const char *show_log_file_mode(void);
You forgot to write the show_log_file_mode()? I was not able to find that
in the patch.
Regards,
--
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
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] gSoC - ADD MERGE COMMAND - code patch submission
Greg Smith writes: > Tom Lane wrote: >> Is there a specific period when that's supposed to happen for GSoC >> students? Can we arrange for a commitfest to be running then > The GSoC "Community bonding period" is described at > http://googlesummerofcode.blogspot.com/2007/04/so-what-is-this-community-bonding-all.html > > and what to cover is a near perfect match for things like introducing > the patch review and submission process. This year, the period from > when proposals were accepted on April 26th through the official coding > start on May 24th were labeled as being devoted to that. Given the way > the release schedule has worked out the last few years, I expect that > every year there will be a whole stack of possibly moldy patches sitting > in the queue for the first CF of the next version at that point. Hmm. Assuming that we manage to keep to an annual release schedule (no sure thing, since we've never done it yet) what that would mean is that the students are looking for feedback while most of the key developers are in heads-down, let's-get-this-release-to-beta mode. Not sure how well that will work. Still, we can try 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
[HACKERS] explain.c: why trace PlanState and Plan trees separately?
Currently, the recursion in ExplainNode() goes to some lengths to chase down the PlanState and Plan trees independently. This is a bit silly: we could just chase the PlanState tree, and use each PlanState's "plan" link when we needed to get to the matching Plan node. I think this is a holdover from long ago when the code worked only with Plan trees --- the PlanState stuff was bolted on rather than replacing that logic entirely. But there is no capacity for EXPLAINing a Plan tree without having constructed a PlanState tree, and I don't foresee that we'd add one (for one reason, EXPLAIN depends on ExecutorStart to perform permissions checking for the referenced tables). Any objections to getting rid of the separate Plan argument? The reason I'm on about this at the moment is that I think I see how to get ruleutils to print PARAM_EXEC Params as the referenced expression rather than $N ... but it depends on having the PlanState tree at hand. So fixing that will destroy any last shred of credibility there might be for EXPLAINing a Plan tree without PlanState. In fact I'm thinking I need to change deparse_context_for_plan() to take a PlanState not a Plan. 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] gSoC - ADD MERGE COMMAND - code patch submission
Tom Lane wrote: Is there a specific period when that's supposed to happen for GSoC students? Can we arrange for a commitfest to be running then The GSoC "Community bonding period" is described at http://googlesummerofcode.blogspot.com/2007/04/so-what-is-this-community-bonding-all.html and what to cover is a near perfect match for things like introducing the patch review and submission process. This year, the period from when proposals were accepted on April 26th through the official coding start on May 24th were labeled as being devoted to that. Given the way the release schedule has worked out the last few years, I expect that every year there will be a whole stack of possibly moldy patches sitting in the queue for the first CF of the next version at that point. I don't think we necessarily need to organize a full on CF around that schedule, but picking a small patch for each student to start chewing on during that period would usefully settle them into list interaction and community development process much more gradually than starting that with their code drops. -- Greg Smith 2ndQuadrant US Baltimore, MD PostgreSQL Training, Services and Support g...@2ndquadrant.com www.2ndQuadrant.us -- 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 - ADD MERGE COMMAND - code patch submission
On Jul 12, 2010, at 4:16 PM, Greg Smith wrote: > I feel the assumption that code is so valuable that it should be shared > regardless of whether it meets conventions is a flawed one for this project. > There are already dozens, if not hundreds, of useful patch submissions that > have been sent to this list, consumed time, and then gone nowhere because > they didn't happen in a way that the community was able to integrate them > properly. True - but we don't want to unduly discourage potential contributors or make them afraid of posting, either. It is for the community to decide whether the effort to clean up a patch is worthwhile, and to provide guidance on what must change. Individual contributors shouldn't seek to take that process off-list, at least IMHO. The main problem with this patch is not that it was submitted as a RAR of multiple diffs against 8.4.3 instead of a single diff against HEAD: it's that we've apparently reached GSoC midterms without making progress beyond what Peter hacked together whilst sitting in an airport. ...Robert -- 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 - ADD MERGE COMMAND - code patch submission
Greg Smith writes: > There is a brief "get to know the community" period at the beginning of > the summer schedule. I think that next year this project would be well > served to give each student a small patch to review during that time, as > a formal intro to the community process. The tendency among students to > just wander off coding without doing any interaction like that is both > common and counterproductive, given how patches to PostgreSQL actually > shuffle along toward becoming commit quality code. Far as I'm > concerned, a day spent working with the patch review checklist on > someone else's patch pays for itself tenfold when it comes time to > produce patches that others will be able to review. That seems like a great idea. Is there a specific period when that's supposed to happen for GSoC students? Can we arrange for a commitfest to be running then? (I guess it'd need to be early in the fest, else the low-hanging fruit will be gone already.) 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] gSoC - ADD MERGE COMMAND - code patch submission
Peter Eisentraut wrote: I think it's better to share code that doesn't mean project guidelines and solicit advice rather than not to share anything. I feel the assumption that code is so valuable that it should be shared regardless of whether it meets conventions is a flawed one for this project. There are already dozens, if not hundreds, of useful patch submissions that have been sent to this list, consumed time, and then gone nowhere because they didn't happen in a way that the community was able to integrate them properly. For anyone who isn't producing commiter quality patches, the process is far more important than the code if you want to get something non-trivial accomplished. Also, producing code in whatever format you want and dumping that on the community so that people like David Fetter waste their time cleaning it up is not the way the GSoC work is supposed to happen. I didn't want any other current or potential future participants in that program to get the wrong idea from that example. There is a brief "get to know the community" period at the beginning of the summer schedule. I think that next year this project would be well served to give each student a small patch to review during that time, as a formal intro to the community process. The tendency among students to just wander off coding without doing any interaction like that is both common and counterproductive, given how patches to PostgreSQL actually shuffle along toward becoming commit quality code. Far as I'm concerned, a day spent working with the patch review checklist on someone else's patch pays for itself tenfold when it comes time to produce patches that others will be able to review. -- Greg Smith 2ndQuadrant US Baltimore, MD PostgreSQL Training, Services and Support g...@2ndquadrant.com www.2ndQuadrant.us -- 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 (for 9.1) string functions
Hello
so this is actualised patch:
* concat_sql removed
* left, right, reverse and concat are in core
* printf and concat_ws are in contrib
* format show "" as NULL string
* removed an using of wide chars
todo:
NULL handling for printf function
Query:
what is corect result for
* printf(">>%3.2d<<", NULL) ??
* printf(">>%3.2s", NULL) ??
Regards
Pavel Stehule
2010/7/12 Itagaki Takahiro :
> 2010/7/12 Robert Haas :
>> I'm all in favor of putting such things in core as are supported by
>> multiple competing products, but is that really true for all of these?
>
> - concat() : MySQL, Oracle, DB2
> - concat_ws() : MySQL,
> - left(), right() : MySQL, SQL Server, DB2
> - reverse() : MySQL, SQL Server, Oracle (as utl_raw.reverse)
>
> concat_sql(), format(), and sprintf() will be our unique features.
>
> --
> Itagaki Takahiro
>
stringfunc.diff
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] gSoC - ADD MERGE COMMAND - code patch submission
On Mon, 2010-07-12 at 23:28 +0300, Peter Eisentraut wrote: > On mån, 2010-07-12 at 10:04 -0400, Greg Smith wrote: > > Wasting the time of everyone in the community by sharing code that > > doesn't mean any of the project guidelines is a very bad idea; please > > don't do that again. > > I think it's better to share code that doesn't mean project guidelines > and solicit advice rather than not to share anything. Agreed. It is great that we have guidelines. We should definitely encourage people to use them. We should also lead, not push people into wanting to use them. Collaboration is good. JD -- PostgreSQL.org Major Contributor Command Prompt, Inc: http://www.commandprompt.com/ - 509.416.6579 Consulting, Training, Support, Custom Development, Engineering -- 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 - ADD MERGE COMMAND - code patch submission
On mån, 2010-07-12 at 10:04 -0400, Greg Smith wrote: > Wasting the time of everyone in the community by sharing code that > doesn't mean any of the project guidelines is a very bad idea; please > don't do that again. I think it's better to share code that doesn't mean project guidelines and solicit advice rather than not to share anything. -- 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] 64-bit pgbench V2
Tom Lane wrote:
Please choose a way that doesn't introduce new portability assumptions.
The backend gets along fine without strtoll, and I don't see why pgbench
should have to require it.
Funny you should mention this...it turns out there is some code already
there, I just didn't notice it before because it's only the unsigned
64-bit strtoul used, not the signed one I was looking for, and it's only
called in one place I didn't previously check.
src/interfaces/ecpg/ecpglib/data.c does this:
*((unsigned long long int *) (var + offset * act_tuple)) =
strtoull(pval, &scan_length, 10);
The appropriate autoconf magic was in the code all along for both
versions, so my bad not noticing it until now. It even transparently
remaps the BSD-ism of calling it strtoq.
I suspect that this alone isn't sufficient to make the code I'm trying
to wedge into pgbench to always work on the platforms I consider must
haves, because of the weird names like _strtoi64 that Windows uses:
http://msdn.microsoft.com/en-us/library/h80404d3(v=VS.80).aspx In fact,
I wouldn't be surprised to discover the ECPG code above doesn't do the
right thing if compiled with a 64-bit MSVC version. Don't expect that's
a popular combination to explicitly test in a way that hits the code
path where this line is at.
The untested (I need to setup for building Windows to really confirm
this works) next patch attempt I've attached does what I think is the
right general sort of thing here. It extends the autoconf remapping
that was already being done to include the second variation on how the
function needed can be named in a MSVC build. This might improve the
ECPG compatibility issue I theorize could be there on that platform.
Given the autoconf stuff and use of the unsigned version was already a
dependency, I'd rather improve that code (so it's more obvious when it
is broken) than do the refactoring work suggested to re-use the server's
internal 64-bit parsing method instead. I could split this into two
patches instead--"add 64-bit strtoull/strtoll support for MSVC" on the
presumption it's actually broken now (possibly wrong on my part) and
"make pgbench use 64-bit values"--but it's not so complicated as one.
I expect there is almost zero overlap between "needs pgbench setshell to
return >32 bit return values" and "not on a platform with a working
64-bit strtoull variation". What I did to hedge against that was add a
little check to pgbench that lets you confirm whether setshell lines are
limited to 32 bits or not, depending on whether the appropriate function
was found. It tries to fall back to the existing strtol in that case,
and I've put a note when that happens (and matching documentation to
look for it) into the debug output of the program.
I'll continue with testing work here, but what's attached is now the
first form I think this could potentially be committed in once it's
known to be free of obvious bugs (testing at this database scale takes
forever). I can revisit not using the library function instead if Tom
or someone else really opposes this new approach. Given most of the
autoconf bits are already there and the limited number of platforms
where this is a problem, I think there's little gain for doing that work
though.
Style/functional suggestions appreciated.
--
Greg Smith 2ndQuadrant US Baltimore, MD
PostgreSQL Training, Services and Support
g...@2ndquadrant.com www.2ndQuadrant.us
diff --git a/configure b/configure
index f6b891e..a5371ba 100755
--- a/configure
+++ b/configure
@@ -21624,7 +21624,8 @@ fi
-for ac_func in strtoll strtoq
+
+for ac_func in strtoll strtoq _strtoi64
do
as_ac_var=`$as_echo "ac_cv_func_$ac_func" | $as_tr_sh`
{ $as_echo "$as_me:$LINENO: checking for $ac_func" >&5
@@ -21726,7 +21727,8 @@ done
-for ac_func in strtoull strtouq
+
+for ac_func in strtoull strtouq _strtoui64
do
as_ac_var=`$as_echo "ac_cv_func_$ac_func" | $as_tr_sh`
{ $as_echo "$as_me:$LINENO: checking for $ac_func" >&5
diff --git a/configure.in b/configure.in
index 0a529fa..cca6453 100644
--- a/configure.in
+++ b/configure.in
@@ -1385,8 +1385,8 @@ if test x"$pgac_cv_var_int_optreset" = x"yes"; then
AC_DEFINE(HAVE_INT_OPTRESET, 1, [Define to 1 if you have the global variable 'int optreset'.])
fi
-AC_CHECK_FUNCS([strtoll strtoq], [break])
-AC_CHECK_FUNCS([strtoull strtouq], [break])
+AC_CHECK_FUNCS([strtoll strtoq _strtoi64], [break])
+AC_CHECK_FUNCS([strtoull strtouq _strtoui64], [break])
# Check for one of atexit() or on_exit()
AC_CHECK_FUNCS(atexit, [],
diff --git a/contrib/pgbench/pgbench.c b/contrib/pgbench/pgbench.c
index c830dee..541510b 100644
--- a/contrib/pgbench/pgbench.c
+++ b/contrib/pgbench/pgbench.c
@@ -56,6 +56,15 @@
#include /* for getrlimit */
#endif
+/*
+ * If this platform doesn't have a 64-bit strtoll, fall back to
+ * using the 32-bit version.
+ */
+#ifndef HAVE_STRTOLL
+#define strtoll strtol
+#define LIMITED_STRTOLL
+#endif
+
#
Re: [HACKERS] Status report on writeable CTEs
On 7/12/10 9:34 PM +0300, Tom Lane wrote: Marko Tiikkaja writes: ... So what I'm now thinking of is making the planner plan that as a single Query, and make the planner expand it into multiple PlannedStmts if necessary. This would break the existing planner hooks, but I don't think that's a huge problem. Does anyone see any obvious flaws in this? How will that interact with the existing rewriter? It sounds a lot like a recipe for duplicating functionality ... I was thinking that the rewriter would look at the top-level CTEs and rewrite all non-SELECT queries into a list of Queries and store that list into the CommonTableExprs. The planner would then use this information and also expand the rewrite products. Regards, Marko Tiikkaja -- 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] Status report on writeable CTEs
Marko Tiikkaja writes: > ... So what I'm now thinking of is making the planner plan that as a single > Query, and make the planner expand it into multiple PlannedStmts if > necessary. This would break the existing planner hooks, but I don't > think that's a huge problem. Does anyone see any obvious flaws in this? How will that interact with the existing rewriter? It sounds a lot like a recipe for duplicating functionality ... 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] Status report on writeable CTEs
On 7/12/10 9:07 PM +0300, I wrote: Consider: WITH t AS (SELECT 1), t2 AS (SELECT * FROM t2) VALUES (true); That should of course have been SELECT * FROM t, not t2. Regards, Marko Tiikkaja -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Status report on writeable CTEs
Hi, I've been working on writeable CTEs during the last couple of months, but right now it looks like I'm going to miss the first commit fest for 9.1. I was trying to make it work by expanding all wCTEs to their own Queries during the rewrite stage (a very crude patch trying to do that for regular CTEs attached), but I don't think that it's a good way of approaching the problem. Consider: WITH t AS (SELECT 1), t2 AS (SELECT * FROM t2) VALUES (true); The first big problem I hit is was that when the query for t2 is planned separately, it doesn't have the CTE information. But even if it had that information (we could easily copy it during the rewrite stage), all RTEs referencing CTEs that were expanded would have the wrong levelsup (this is where the patch fails at regression tests). One could probably come up with ways of fixing even that, but I don't think that's the right direction to be heading. So what I'm now thinking of is making the planner plan that as a single Query, and make the planner expand it into multiple PlannedStmts if necessary. This would break the existing planner hooks, but I don't think that's a huge problem. Does anyone see any obvious flaws in this? Any feedback is welcome. I'd also be happy to get some help on this project. Regards, Marko Tiikkaja *** a/src/backend/commands/copy.c --- b/src/backend/commands/copy.c *** *** 1092,1098 DoCopy(const CopyStmt *stmt, const char *queryString) cstate->queryDesc = CreateQueryDesc(plan, queryString, GetActiveSnapshot(), InvalidSnapshot, ! dest, NULL, 0); /* * Call ExecutorStart to prepare the plan for execution. --- 1092,1098 cstate->queryDesc = CreateQueryDesc(plan, queryString, GetActiveSnapshot(), InvalidSnapshot, ! dest, NULL, 0, NIL); /* * Call ExecutorStart to prepare the plan for execution. *** a/src/backend/commands/explain.c --- b/src/backend/commands/explain.c *** *** 367,373 ExplainOnePlan(PlannedStmt *plannedstmt, ExplainState *es, /* Create a QueryDesc requesting no output */ queryDesc = CreateQueryDesc(plannedstmt, queryString, GetActiveSnapshot(), InvalidSnapshot, ! None_Receiver, params, instrument_option); INSTR_TIME_SET_CURRENT(starttime); --- 367,374 /* Create a QueryDesc requesting no output */ queryDesc = CreateQueryDesc(plannedstmt, queryString, GetActiveSnapshot(), InvalidSnapshot, ! None_Receiver, params, instrument_option, ! NIL); INSTR_TIME_SET_CURRENT(starttime); *** *** 692,697 ExplainNode(Plan *plan, PlanState *planstate, --- 693,701 case T_CteScan: pname = sname = "CTE Scan"; break; + case T_DtScan: + pname = sname = "Derived Table Scan"; + break; case T_WorkTableScan: pname = sname = "WorkTable Scan"; break; *** *** 844,849 ExplainNode(Plan *plan, PlanState *planstate, --- 848,854 case T_ValuesScan: case T_CteScan: case T_WorkTableScan: + case T_DtScan: ExplainScanTarget((Scan *) plan, es); break; case T_BitmapIndexScan: *** *** 1565,1570 ExplainScanTarget(Scan *plan, ExplainState *es) --- 1570,1581 objectname = rte->ctename; objecttag = "CTE Name"; break; + case T_DtScan: + /* Assert it's on a non-self-reference CTE */ + Assert(rte->rtekind == RTE_CTE); + Assert(!rte->self_reference); + objectname = rte->ctename; + objecttag = "Derived Table Name"; default: break; } *** a/src/backend/executor/Makefile --- b/src/backend/executor
Re: [HACKERS] patch (for 9.1) string functions
2010/7/12 Kevin Grittner :
> Itagaki Takahiro wrote:
>
>> I'd like to move all proposed functions into the core, and not to
>> add contrib/stringfunc.
>
>> Still failed :-( I used UTF8 database with *locale=C* on 64bit
>> Linux.
>> char2wchar() doesn't seem to work on C locale. We should avoid
>> using the function and converting mb chars to wide chars.
>>
>> select sprintf('>>>%10s %10d<<<', 'hello', 10);
>> ! server closed the connection unexpectedly
>> TRAP: FailedAssertion("!(!lc_ctype_is_c())", File: "mbutils.c",
>> Line: 715)
>>
>> #0 0x0038c0c332f5 in raise () from /lib64/libc.so.6
>> #1 0x0038c0c34b20 in abort () from /lib64/libc.so.6
>> #2 0x006e951d in ExceptionalCondition
>> (conditionName=, errorType=> out>, fileName=, lineNumber=> out>) at assert.c:57
>> #3 0x006fa8bf in char2wchar (to=0x1daf188 L"", tolen=16,
>> from=0x1da95b0 "%*s", fromlen=3) at mbutils.c:715
>> #4 0x7f8e8c436d37 in stringfunc_sprintf
>> (fcinfo=0x7fff9bdcd4b0)
>> at stringfunc.c:463
>
> Based on this and subsequent posts, I've changed this patch's status
> to "Waiting on Author".
ook - I'll send actualised version tomorrow
Pavel
>
> -Kevin
>
--
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 (for 9.1) string functions
Itagaki Takahiro wrote:
> I'd like to move all proposed functions into the core, and not to
> add contrib/stringfunc.
> Still failed :-( I used UTF8 database with *locale=C* on 64bit
> Linux.
> char2wchar() doesn't seem to work on C locale. We should avoid
> using the function and converting mb chars to wide chars.
>
> select sprintf('>>>%10s %10d<<<', 'hello', 10);
> ! server closed the connection unexpectedly
> TRAP: FailedAssertion("!(!lc_ctype_is_c())", File: "mbutils.c",
> Line: 715)
>
> #0 0x0038c0c332f5 in raise () from /lib64/libc.so.6
> #1 0x0038c0c34b20 in abort () from /lib64/libc.so.6
> #2 0x006e951d in ExceptionalCondition
> (conditionName=, errorType= out>, fileName=, lineNumber= out>) at assert.c:57
> #3 0x006fa8bf in char2wchar (to=0x1daf188 L"", tolen=16,
> from=0x1da95b0 "%*s", fromlen=3) at mbutils.c:715
> #4 0x7f8e8c436d37 in stringfunc_sprintf
> (fcinfo=0x7fff9bdcd4b0)
> at stringfunc.c:463
Based on this and subsequent posts, I've changed this patch's status
to "Waiting on Author".
-Kevin
--
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 - snapshot materialized view (work-in-progress) patch
Pavel Baroš wrote: > Dne 9.7.2010 21:33, Robert Haas napsal(a): >> Please add your patch here, so that it will be reviewed during >> the about-to-begin CommitFest. >> >> https://commitfest.postgresql.org/action/commitfest_view/open >> > > OK, but will you help me with that form? Do you think I can fill > it like that? I'm not sure about few fields .. > > Name: Snapshot materialized views > CommitFest Topic: [ Miscellaneous | SQL Features ] ??? SQL Features seems reasonable to me. > Patch Status: Needs review > Author: me > Reviewers:You? Leave empty. Reviewers will sign up or be assigned. > Commiters:who? That comes much later -- when the patch is complete and has a favorable review, then a committer will pick it up. > and I quess fields 'Date Closed' and 'Message-ID for Original > Patch' will be filled later. Date closed is only set for patches which are committed, returned with feedback (for a later CommitFest), or rejected. When you make an entry which references a post to the lists, you should fill in the Message-ID from the email header of the post. You may be able to get this from your email software as soon as you send the post; if not, you can find it on the archive page for the post. -Kevin -- 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 - snapshot materialized view (work-in-progress) patch
Dne 9.7.2010 21:33, Robert Haas napsal(a): 2010/7/8 Pavel Baroš: Description of patch: 1) can create MV, and is created uninitialized with data CREATE MATERIALIZED VIEW mvname AS SELECT ... This doesn't seem acceptable. It should populate it on creation. Yes, it would be better, in addition, true is, this behavior will be required if is expected to implement incremental MV in the close future. 2) can refresh MV ALTER MATERIALIZED VIEW mvname REFRESH 3) MV cannot be modified by DML commands (INSERT, UPDATE and DELETE are not permitted) 4) index can be created and used with MV 5) pg_dump is repaired, in previous patch dump threw error, now dont, but it is sort of dummy, I want to reach state, where refreshing command will be posed after all COPY statements (when all data are in tables). In this patch REFRESH command is right behind CREATE MV command. Hmm... ISTM that you probably need some kind of dependency stuff in here to make the materialized view get created after the tables it depends on have been populated with data. It needs to work with parallel restore, too. I'm not sure exactly how the dependency stuff in pg_dump works, though. never mind in case MV will be populated on creation. A subtle point here is that if you dump and restore a database containing a materialized view, the new database might not be quite the same as the old one, because the materialized view might have been out of date before, and when you recreate it, it'll get refreshed. I'm not sure there's much we can/should do about that, though. yes, it is interesting, of course, there can be real-life example, where population on creating is needed and is not, and I'm thinking of solution similar to Oracle or DB2. Add some option to creating MV, that enable/disable population on creating: http://www.ibm.com/developerworks/data/library/techarticle/dm-0708khatri/ Oracle: CREATE MATERIALIZED VIEW mvname [ BUILD [IMMEDIATE | DEFERRED] ] AS SELECT .. DB2: CREATE TABLE mvname AS SELECT ... [ INITIALLY DEFERRED | IMMEDIATE ] 6) psql works too, new command \dm[S+] was added to the list \d[S+] [PATTERN] - lists all db objects like tables, view, materialized view and sequences \dm[S+] [PATTERN] - lists all materialized views I also noticed I forgot handle options \dp and \dpp, this should be OK in next version of patch. 7) there are some docs too, but I guess it is not enough, at least my english will need to correct If we're going to treat materialized views as a separate object type, you probably need to break out the docs for CREATE MATERIALIZED VIEW, ALTER MATERIALIZED VIEW, and DROP MATERIALIZED VIEW into their own pages, rather than having then mixed up with corresponding pages for regular views. Yeah, that was problem I just solved like that here, but I confess this would be better. In progress: - regression tests - behavior of various ALTER commands, ie SET STATISTIC, CLUSTER ON, ENABLE/DISABLE RULE, etc. This isn't right: rhaas=# create view v as select * from t; CREATE VIEW rhaas=# alter view v refresh; ERROR: unrecognized alter table type: 41 I know, cases like that will be more than that. Thats why I work on good tests now. Please add your patch here, so that it will be reviewed during the about-to-begin CommitFest. https://commitfest.postgresql.org/action/commitfest_view/open OK, but will you help me with that form? Do you think I can fill it like that? I'm not sure about few fields .. Name: Snapshot materialized views CommitFest Topic: [ Miscellaneous | SQL Features ] ??? Patch Status: Needs review Author: me Reviewers:You? Commiters:who? and I quess fields 'Date Closed' and 'Message-ID for Original Patch' will be filled later. thanks a lot Pavel Baros
Re: [HACKERS] - GSoC - snapshot materialized view (work-in-progress) patch
Dne 9.7.2010 21:33, Robert Haas napsal(a): 2010/7/8 Pavel Baroš: Description of patch: 1) can create MV, and is created uninitialized with data CREATE MATERIALIZED VIEW mvname AS SELECT ... This doesn't seem acceptable. It should populate it on creation. Yes, it would be better, in addition, true is, this behavior will be required if is expected to implement incremental MV in the close future. 2) can refresh MV ALTER MATERIALIZED VIEW mvname REFRESH 3) MV cannot be modified by DML commands (INSERT, UPDATE and DELETE are not permitted) 4) index can be created and used with MV 5) pg_dump is repaired, in previous patch dump threw error, now dont, but it is sort of dummy, I want to reach state, where refreshing command will be posed after all COPY statements (when all data are in tables). In this patch REFRESH command is right behind CREATE MV command. Hmm... ISTM that you probably need some kind of dependency stuff in here to make the materialized view get created after the tables it depends on have been populated with data. It needs to work with parallel restore, too. I'm not sure exactly how the dependency stuff in pg_dump works, though. never mind in case MV will be populated on creation. A subtle point here is that if you dump and restore a database containing a materialized view, the new database might not be quite the same as the old one, because the materialized view might have been out of date before, and when you recreate it, it'll get refreshed. I'm not sure there's much we can/should do about that, though. yes, it is interesting, of course, there can be real-life example, where population on creating is needed and is not, and I'm thinking of solution similar to Oracle or DB2. Add some option to creating MV, that enable/disable population on creating: http://www.ibm.com/developerworks/data/library/techarticle/dm-0708khatri/ Oracle: CREATE MATERIALIZED VIEW mvname [ BUILD [IMMEDIATE | DEFERRED] ] AS SELECT .. DB2: CREATE TABLE mvname AS SELECT ... [ INITIALLY DEFERRED | IMMEDIATE ] 6) psql works too, new command \dm[S+] was added to the list \d[S+] [PATTERN] - lists all db objects like tables, view, materialized view and sequences \dm[S+] [PATTERN] - lists all materialized views I also noticed I forgot handle options \dp and \dpp, this should be OK in next version of patch. 7) there are some docs too, but I guess it is not enough, at least my english will need to correct If we're going to treat materialized views as a separate object type, you probably need to break out the docs for CREATE MATERIALIZED VIEW, ALTER MATERIALIZED VIEW, and DROP MATERIALIZED VIEW into their own pages, rather than having then mixed up with corresponding pages for regular views. Yeah, that was problem I just solved like that here, but I confess this would be better. In progress: - regression tests - behavior of various ALTER commands, ie SET STATISTIC, CLUSTER ON, ENABLE/DISABLE RULE, etc. This isn't right: rhaas=# create view v as select * from t; CREATE VIEW rhaas=# alter view v refresh; ERROR: unrecognized alter table type: 41 I know, cases like that will be more than that. Thats why I work on good tests now. Please add your patch here, so that it will be reviewed during the about-to-begin CommitFest. https://commitfest.postgresql.org/action/commitfest_view/open OK, but will you help me with that form? Do you think I can fill it like that? I'm not sure about few fields .. Name: Snapshot materialized views CommitFest Topic: [ Miscellaneous | SQL Features ] ??? Patch Status: Needs review Author: me Reviewers:You? Commiters:who? and I quess fields 'Date Closed' and 'Message-ID for Original Patch' will be filled later. thanks a lot Pavel Baros -- 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 - ADD MERGE COMMAND - code patch submission
Boxuan Zhai wrote: I found that people have problems on running my codes, which probably comes from my nonstandard submission format. I can compile, install and initialize postgres in my own machine. The system accepts MERGE command and will throw an error when it runs into the executor, which cannot recognize the MERGE command type so far. Your job as a potential contributor to PostgreSQL is to make it as easy as possible for others to test your code out and get good results. I sent you some more detailed guidelines over the weekend as to what I think you should do here to achieve that. You should wait until you've gotten a private review from one of the two people who have volunteered to help you out here before you submit anything else to the list. Wasting the time of everyone in the community by sharing code that doesn't mean any of the project guidelines is a very bad idea; please don't do that again. -- Greg Smith 2ndQuadrant US Baltimore, MD PostgreSQL Training, Services and Support g...@2ndquadrant.com www.2ndQuadrant.us -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] CommitFest 2010-07 Plans and Call for Reviewers
Folks, The PostgreSQL 9.1 Development Plan: http://wiki.postgresql.org/wiki/PostgreSQL_9.1_Development_Plan calls for a CommitFest to run from the 15th of July until the 15th of August. I've offered to manage the CF process this time around. (Selena, are you up for continuing to work on this through the CF, too?) I was reluctant to push very hard on reviewers during the ReviewFest leading up to the CF, for fear of impacting the release, but I've taken Robert's words from this post to heart: http://archives.postgresql.org/pgsql-rrreviewers/2010-07/msg2.php In particular: > In terms of being aggressive about pursuing this, I think it's > important that between July 15th and August 14th we try to give > everyone who has submitted a patch by July 14th some feedback on > their work, which means we need more people reviewing than have so > far. I don't think the release will be out before the CF is over, > but I'm hoping that we can get enough people involved to walk and > chew gum at the same time. As Robert point out, we need more reviewers. Before signing up, please review these pages, to get an idea what's involved: http://wiki.postgresql.org/wiki/Reviewing_a_Patch http://wiki.postgresql.org/wiki/RRReviewers On the lighter side: http://wiki.postgresql.org/images/5/58/11_eggyknap-patch-review.pdf Please send me an email (without copying the list) if you are available to review; feel free to include any information that might be helpful in assigning you an appropriate patch. To summarize where we are now: 54 patches are listed in the CF web page. Of those, 4 have already been committed, 3 have been returned with feedback, and 1 has been rejected -- leaving 46 patches active. Of these 8 are ready for committer and 6 are waiting on author. That leaves 32 which currently need review, of which 22 don't yet have anyone assigned (or volunteered) to do the review. -Kevin -- 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 (for 9.1) string functions
On Jul 11, 2010, at 10:32 PM, Itagaki Takahiro wrote: > 2010/7/12 Robert Haas : >> I'm all in favor of putting such things in core as are supported by >> multiple competing products, but is that really true for all of these? > > - concat() : MySQL, Oracle, DB2 > - concat_ws() : MySQL, > - left(), right() : MySQL, SQL Server, DB2 > - reverse() : MySQL, SQL Server, Oracle (as utl_raw.reverse) > > concat_sql(), format(), and sprintf() will be our unique features. I think concat, left, right, and reverse should go into core, and perhaps format also. The rest sound like contrib material to me. ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [PATCH] Re: [HACKERS] Issue: Deprecation of the XML2 module 'xml_is_well_formed' function
On 12 July 2010 13:07, Mike Fowler wrote: > Thom Brown wrote: >> >> Just wondering about that semi-colon after the namespace definition. >> >> Thom >> > > The semi-colon is not supposed to be there, and I'm not sure where it's come > from. With Thunderbird I see the email with my patch as an attachement, > downloaded and viewing the file there are no instances of a " followed by a > ;. However, if I look at the message on the archive at > http://archives.postgresql.org/message-id/4c3871c2.8000...@mlfowler.com I > can see every URL that ends with a " has a ; following it. Should I be > escaping the " in the patch file in some way or this just an artifact of > HTML parsing a patch? Yeah, I guess it's a parsing issue related to the archive viewer. I arrived there from the commitfest page and should have really looked directly at the patch. No problem there then I guess. Thanks for the work you've done on this. :) Thom -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [PATCH] Re: [HACKERS] Issue: Deprecation of the XML2 module 'xml_is_well_formed' function
Thom Brown wrote:
Would a test for mismatched or undefined namespaces be necessary?
For example:
Mismatched namespace:
http://postgresql.org/stuff";>bar
Undefined namespace when used in conjunction with IS DOCUMENT:
http://postgresql.org/stuff";>bar
Thanks for looking at my patch Thom. I hadn't thought of that particular
scenario and even though I didn't specifically code for it, the
underlying libxml call does correctly reject the mismatched namespace:
template1=# SELECT xml_is_well_formed('http://postgresql.org/stuff";>bar');
xml_is_well_formed
f
(1 row)
In the attached patch I've added the example to the SGML documentation
and the regression tests.
Also, having a look at the following example from the patch:
SELECT xml_is_well_formed('http://127.0.0.1";;>number
one');
xml_is_well_formed
t
(1 row)
Just wondering about that semi-colon after the namespace definition.
Thom
The semi-colon is not supposed to be there, and I'm not sure where it's
come from. With Thunderbird I see the email with my patch as an
attachement, downloaded and viewing the file there are no instances of a
" followed by a ;. However, if I look at the message on the archive at
http://archives.postgresql.org/message-id/4c3871c2.8000...@mlfowler.com
I can see every URL that ends with a " has a ; following it. Should I
be escaping the " in the patch file in some way or this just an artifact
of HTML parsing a patch?
Regards,
--
Mike Fowler
Registered Linux user: 379787
*** a/contrib/xml2/xpath.c
--- b/contrib/xml2/xpath.c
***
*** 27,33 PG_MODULE_MAGIC;
/* externally accessible functions */
- Datum xml_is_well_formed(PG_FUNCTION_ARGS);
Datum xml_encode_special_chars(PG_FUNCTION_ARGS);
Datum xpath_nodeset(PG_FUNCTION_ARGS);
Datum xpath_string(PG_FUNCTION_ARGS);
--- 27,32
***
*** 70,97 pgxml_parser_init(void)
xmlLoadExtDtdDefaultValue = 1;
}
-
- /* Returns true if document is well-formed */
-
- PG_FUNCTION_INFO_V1(xml_is_well_formed);
-
- Datum
- xml_is_well_formed(PG_FUNCTION_ARGS)
- {
- text *t = PG_GETARG_TEXT_P(0); /* document buffer */
- int32 docsize = VARSIZE(t) - VARHDRSZ;
- xmlDocPtr doctree;
-
- pgxml_parser_init();
-
- doctree = xmlParseMemory((char *) VARDATA(t), docsize);
- if (doctree == NULL)
- PG_RETURN_BOOL(false); /* i.e. not well-formed */
- xmlFreeDoc(doctree);
- PG_RETURN_BOOL(true);
- }
-
-
/* Encodes special characters (<, >, &, " and \r) as XML entities */
PG_FUNCTION_INFO_V1(xml_encode_special_chars);
--- 69,74
*** a/doc/src/sgml/func.sgml
--- b/doc/src/sgml/func.sgml
***
*** 8554,8562 SELECT xmlagg(x) FROM (SELECT * FROM test ORDER BY y DESC) AS tab;
]]>
! XML Predicates
IS DOCUMENT
--- 8554,8566
]]>
+
+
+
+XML Predicates
! IS DOCUMENT
IS DOCUMENT
***
*** 8574,8579 SELECT xmlagg(x) FROM (SELECT * FROM test ORDER BY y DESC) AS tab;
--- 8578,8675
between documents and content fragments.
+
+
+ xml_is_well_formed
+
+
+ xml_is_well_formed
+ well formed
+
+
+
+ xml_is_well_formed(text)
+
+
+
+ The function xml_is_well_formed evaluates whether
+ the text is well formed XML content, returning
+ a boolean.
+
+
+ Example:
+
+
+
+ In addition to the structure checks, the function ensures that namespaces are correcty matched.
+
+
+
+ This function can be combined with the IS DOCUMENT predicate to prevent
+ invalid XML content errors from occuring in queries. For example, given a
+ table that may have rows with invalid XML mixed in with rows of valid
+ XML, xml_is_well_formed can be used to filter out all
+ the invalid rows.
+
+
+ Example:
+
+
+
*** a/src/backend/utils/adt/xml.c
--- b/src/backend/utils/adt/xml.c
***
*** 3293,3298 xml_xmlnodetoxmltype(xmlNodePtr cur)
--- 3293,3365
}
#endif
+ Datum
+ xml_is_well_formed(PG_FUNCTION_ARGS)
+ {
+ #ifdef USE_LIBXML
+ text*data = PG_GETARG_TEXT_P(0);
+ boolresult;
+ int res_code;
+ int32len;
+ const xmlChar *string;
+ xmlParserCtxtPtr ctxt;
+ xmlDocPtr doc = NULL;
+
+ len = VARSIZE(data) - VARHDRSZ;
+ string = xml_text2xmlChar(data);
+
+ /* Start up libxml and its parser (no-ops if already done) */
+ pg_xml_init();
+ xmlInitParser();
+
+ ctxt = xmlNewParserCtxt();
+ if (ctxt == NULL)
+ xml_ereport(ERROR, ERRCODE_OUT_OF_MEMORY,
+ "could not allocate parser context");
+
+ PG_TRY();
+ {
+ size_t count;
+ xmlChar*version = NULL;
+ int standalone = -1;
+
+ res_code = parse_xml_decl(string, &count, &version, NULL, &standalone);
+ if (res_code != 0)
+ xml_ereport_by_code(ERROR, ERRCODE_INVALID_XML_CONTENT,
+
Re: [HACKERS] log files and permissions
Itagaki Takahiro wrote:
> I checked "log_file_mode GUC" patch, and found a couple of Windows-specific
> and translation issues.
Thank you for the review. Attached patch attempts to fix these issues.
> * fchmod() is not available on some platforms, including Windows.
> fh = fopen(filename, mode);
> setvbuf(fh, NULL, LBF_MODE, 0);
> fchmod(fileno(fh), Log_file_mode);
>
I've changed that to using chmod(), that should be available everywhere and
is already used in several places in Postgres code.
> * How does the file mode work on Windows?
> If it doesn't work, we should explain it in docs.
Indeed it seems that chmod() doesn't actually do anything useful on Windows.
So I've added a documentation note about it and put an #ifndef WIN32 around
the chmod() call.
> It might look a duplication of codes, but I think this form is better
> because we can reuse the existing translation catalogs.
> if (am_rotating)
> ereport(FATAL, ... "could not create log file ...);
> else
> ereport(LOG, ... "could not open new log file ...);
>
Fixed.
regards,
Martin
*** a/doc/src/sgml/config.sgml
--- b/doc/src/sgml/config.sgml
***
*** 2789,2794 local0.*/var/log/postgresql
--- 2789,2814
+
+ log_file_mode (integer)
+
+log_file_mode configuration parameter
+
+
+
+ On Unix systems this parameter sets the permissions for log files
+ when logging_collector is enabled. On Microsoft
+ Windows the file mode will be ignored. The value is an octal number
+ consisting of 3 digits signifying the permissions for the user, group
+ and others.
+
+
+ This parameter can only be set in the postgresql.conf
+ file or on the server command line.
+
+
+
+
log_rotation_age (integer)
*** a/src/backend/commands/variable.c
--- b/src/backend/commands/variable.c
***
*** 914,916 show_role(void)
--- 914,946
return endptr + 1;
}
+
+
+ /*
+ * LOG_FILE_MODE
+ */
+
+ extern int Log_file_mode; /* in guc.c */
+
+ /*
+ * assign_log_file_mode: GUC assign_hook for log_file_mode
+ */
+ const char *
+ assign_log_file_mode(const char *value, bool doit, GucSource source)
+ {
+ char *endptr;
+ long file_mode = strtol(value, &endptr, 8);
+
+ if (!*value || *endptr || file_mode < 0 || file_mode > 0777)
+ {
+ ereport(GUC_complaint_elevel(source),
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("invalid value for parameter \"log_file_mode\"")));
+ return NULL;
+ }
+
+ if (doit)
+ Log_file_mode = (int) file_mode;
+
+ return value;
+ }
*** a/src/backend/postmaster/syslogger.c
--- b/src/backend/postmaster/syslogger.c
***
*** 73,78 int Log_RotationSize = 10 * 1024;
--- 73,79
char *Log_directory = NULL;
char *Log_filename = NULL;
bool Log_truncate_on_rotation = false;
+ int Log_file_mode = 0600;
/*
* Globally visible state (used by elog.c)
***
*** 135,140 static void syslogger_parseArgs(int argc, char *argv[]);
--- 136,142
static void process_pipe_input(char *logbuffer, int *bytes_in_logbuffer);
static void flush_pipe_input(char *logbuffer, int *bytes_in_logbuffer);
static void open_csvlogfile(void);
+ static FILE *logfile_open(const char *filename, const char *mode, bool am_rotating);
#ifdef WIN32
static unsigned int __stdcall pipeThread(void *arg);
***
*** 516,530 SysLogger_Start(void)
*/
filename = logfile_getname(time(NULL), NULL);
! syslogFile = fopen(filename, "a");
!
! if (!syslogFile)
! ereport(FATAL,
! (errcode_for_file_access(),
! (errmsg("could not create log file \"%s\": %m",
! filename;
!
! setvbuf(syslogFile, NULL, LBF_MODE, 0);
pfree(filename);
--- 518,524
*/
filename = logfile_getname(time(NULL), NULL);
! syslogFile = logfile_open(filename, "a", false);
pfree(filename);
***
*** 1004,1018 open_csvlogfile(void)
filename = logfile_getname(time(NULL), ".csv");
! fh = fopen(filename, "a");
!
! if (!fh)
! ereport(FATAL,
! (errcode_for_file_access(),
! (errmsg("could not create log file \"%s\": %m",
! filename;
!
! setvbuf(fh, NULL, LBF_MODE, 0);
#ifdef WIN32
_setmode(_fileno(fh), _O_TEXT); /* use CRLF line endings on Windows */
--- 998,1004
filename = logfile_getname(time(NULL), ".csv");
! fh = logfile_open(filename, "a", false);
#ifdef WIN32
_setmode(_fileno(fh), _O_TEXT); /* use CRLF line endings on Windows */
***
*** 1025,1030 open_csvlogfile(void)
--- 1011,1050
}
/*
+ * Open the logfile, set permissions and buffering options.
+ */
+ static FILE *
+ logfile_open(const char *filename, const char *mode, bool am_rotating)
+ {
+ FILE *fh;
+
+ fh = fopen(filename, mode)
Re: [HACKERS] Re: [COMMITTERS] pgsql: Add support for TCP keepalives on Windows, both for backend and
On Sun, Jul 11, 2010 at 00:05, Peter Eisentraut wrote: > On lör, 2010-07-10 at 16:23 -0400, Bruce Momjian wrote: >> Wow, how would they know if the binaries are MinGW compiled? Does it >> show in version()? > > Yes, I think so. It definitely does. -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.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] pg_stat_transaction patch
On Mon, Jul 12, 2010 at 11:36, Itagaki Takahiro wrote: > "Accessor functions to get so far collected statistics for the current > transaction" > https://commitfest.postgresql.org/action/patch_view?id=301 > > The latest version of the patch works as expected, and also well-formed. > I'll mark the patch to "Ready for Committer". > http://archives.postgresql.org/message-id/aanlktiksydrwatgap5u6o6zwio4b_wnjxibud2y2t...@mail.gmail.com > > The added views and functions only accumulates activity counters > in the same transaction where the stat views are queried. So we > need to check the view before COMMIT or ROLLBACK. > > The only issue in the patch is too long view and function names: > - pg_stat_transaction_user_tables (31 chars) > - pg_stat_get_transaction_tuples_hot_updated (42 chars) > - pg_stat_get_transaction_function_self_time (42 chars) > > Since we've already used _xact_ in some system objects, we could replace > _transaction_ parts with _xact_. It will save 7 key types per query ;-) +1 on shortening that down, they're scary long now :-) -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.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] pg_stat_transaction patch
"Accessor functions to get so far collected statistics for the current transaction" https://commitfest.postgresql.org/action/patch_view?id=301 The latest version of the patch works as expected, and also well-formed. I'll mark the patch to "Ready for Committer". http://archives.postgresql.org/message-id/aanlktiksydrwatgap5u6o6zwio4b_wnjxibud2y2t...@mail.gmail.com The added views and functions only accumulates activity counters in the same transaction where the stat views are queried. So we need to check the view before COMMIT or ROLLBACK. The only issue in the patch is too long view and function names: - pg_stat_transaction_user_tables (31 chars) - pg_stat_get_transaction_tuples_hot_updated (42 chars) - pg_stat_get_transaction_function_self_time (42 chars) Since we've already used _xact_ in some system objects, we could replace _transaction_ parts with _xact_. It will save 7 key types per query ;-) -- Itagaki Takahiro -- 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 (for 9.1) string functions
Hello 2010/7/12 Robert Haas : > On Sun, Jul 11, 2010 at 10:30 PM, Itagaki Takahiro > wrote: >> 2010/7/9 Pavel Stehule : >>> I am sending a actualised patch >>> * removed concat_json >>> * renamed function rvsr to reverse >>> * functions format, sprintf and concat* are stable now (as to_char for >>> example) >> >> I'd like to move all proposed functions into the core, and not to add >> contrib/stringfunc. >> I think those functions are very useful and worth adding in core. >> * concat(), concat_ws(), reverse(), left() and right() are ready to commit. >> * format() is almost ready, except consensus of NULL representation. what solution for this moment - be a consistent with RAISE statement ??? >> * sprintf() is also useful, but we cannot use swprintf() in it because >> there are many problems in converting to wide chars. We should >> develop mbchar-aware version of %s formatter. ook I'll work on this - but there is same problem with NULL like a format function >> * IMHO, concat_sql() has very limited use cases. Boolean and numeric >> values are not quoted, but still need product-specific conversions because >> some DBs prefer 1/0 instead of true/false. >> Also, dblink_build_sql_insert() provides similar functionality. Will >> we have both? > I can remove it - when I checked it I found so it doesn't well serialize PostgreSQL specific types as array or record, so I am not against to remove it now. > I'm all in favor of putting such things in core as are supported by > multiple competing products, but is that really true for all of these? > I have not a strong opinion on this - I would to like see reverse and format in core. But I think, so contrib is enought. Can somebody from commiters to decide it, please? Any sprintf implemenation needs lots of code - minimally for this function I prefer contrib for this function. Regards Pavel Stehule > -- > Robert Haas > EnterpriseDB: http://www.enterprisedb.com > The Enterprise Postgres Company > -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [PATCH] Re: [HACKERS] Issue: Deprecation of the XML2 module 'xml_is_well_formed' function
On 10 July 2010 14:12, Mike Fowler wrote:
> Robert Haas wrote:
>>
>> On Fri, Jul 9, 2010 at 4:06 PM, Peter Eisentraut wrote:
>>
>>>
>>> On ons, 2010-07-07 at 16:37 +0100, Mike Fowler wrote:
>>>
Here's the patch to add the 'xml_is_well_formed' function.
>>>
>>> I suppose we should remove the function from contrib/xml2 at the same
>>> time.
>>>
>>
>> Yep
>
> Revised patch deleting the contrib/xml2 version of the function attached.
>
> Regards,
>
> --
> Mike Fowler
> Registered Linux user: 379787
>
sql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>
>
Would a test for mismatched or undefined namespaces be necessary?
For example:
Mismatched namespace:
http://postgresql.org/stuff";>bar
Undefined namespace when used in conjunction with IS DOCUMENT:
http://postgresql.org/stuff";>bar
Also, having a look at the following example from the patch:
SELECT xml_is_well_formed('http://127.0.0.1";;>number
one');
xml_is_well_formed
t
(1 row)
Just wondering about that semi-colon after the namespace definition.
Thom
--
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: to_string, to_array functions
2010/7/12 Itagaki Takahiro : > 2010/7/12 Pavel Stehule : >> I prefere a new names - because there are a new behave - with little >> bit better default handling of NULL values. string_to_array and >> array_to_string just ignore NULL values - what isn't correct behave. >> Later we can mark these functions as deprecated and remove it. If I >> use current function, then we have to continue in current behave. > > I prefer existing names because your new default behavior can be done > with suitable nullstr values. IMHO, new names will be acceptable only if > they are listed in the SQL-standard or many other databases use the > names. Two similar versions of functions must confuse users. there is different default behave. So if you don't need to use a third argument > > Also, are there any consensus about "existing functions are not correct" ? > Since string_agg() and your new concat() functions ignores NULLs, > I think it is not so bad for array_to_string() to ignore NULLs. string_agg is a aggregate function - there are NULLS ignored usually, concat simulate MySQL behave - and more, there are not problem to use a coalesce function. string_to_arrays and array_to string are different - there you cannot use a coalesce. Why string_to_array and array_to_strings are not correct? a) what is correct sample of using a array_to_string with NULL ignoring?? Usually, when you have a NULL in array, you don't want to loose this value. b) for me - these functions are some of serialisation/deserialisation functions - usually people don't want to miss any value. I searching in history - my first proposal was similar to your: http://www.mail-archive.com/pgsql-hackers@postgresql.org/msg151474.html http://www.mail-archive.com/pgsql-hackers@postgresql.org/msg151503.html !! if you look on this thread, you can see so I was unsure and confused too - but now I inclinded to Merlin's proposal shortly: * string_to_array/array_to_string ignore nulls * others not aggregates not ignore nulls * default for NULL isn't "NULL" but empty string - like csv regards Pavel Stěhule > > -- > Itagaki Takahiro > -- 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] (9.1) btree_gist support for searching on "not equals"
(1) Exclusion constraints support for operators where "x x"
is false (tiny patch)
https://commitfest.postgresql.org/action/patch_view?id=307
(2) btree_gist support for searching on <> ("not equals")
https://commitfest.postgresql.org/action/patch_view?id=308
Those patches should be committed at once because (2) requires (1) to work
with EXCLUDE constraints. Also, (1) has no benefits without (2) because we
have no use cases for <> as an index-able operator. Both patches are very
simple and small, and worked as expected both "WHERE <>" and EXCLUDE
constraints cases.
I'd like to ask you to write additional documentation about btree_gist [1]
that the module will be more useful when it is used with exclusion
constraints together. Without documentation, no users find the usages.
Of course the docs can be postponed if you have a plan to write docs
when PERIOD types are introduced,
[1] http://developer.postgresql.org/pgdocs/postgres/btree-gist.html
The patch was not applied to 9.0, but the reason was just "no time to test" [2].
We have enough time to test for 9.1, so we can apply it now!
[2] http://archives.postgresql.org/pgsql-hackers/2010-05/msg01874.php
--
Itagaki Takahiro
--
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: to_string, to_array functions
some note 2010/7/12 Pavel Stehule : > 2010/7/12 Itagaki Takahiro : >> https://commitfest.postgresql.org/action/patch_view?id=300 >> >> Why did you add to_string() and to_array() functions though we already >> have string_to_array() and array_to_string() functions? I prefer adding >> three arguments version of string_to_array() instead of to_array(). >> Please notice me if you think to_string() and to_array() are better names >> for the feature. For example, compatibility for other databases. >> > > I prefere a new names - because there are a new behave - with little > bit better default handling of NULL values. string_to_array and > array_to_string just ignore NULL values - what isn't correct behave. it is related to time where pg arrays doesn't support a NULL. From 8.3 pg array can have a NULL values, but there wasn't any equal changes to string_to_array and array_to_string functions - so these functions are not "actual". pavel > Later we can mark these functions as deprecated and remove it. If I > use current function, then we have to continue in current behave. > >> * string_to_array( str text, sep text, nullstr text DEFAULT NULL ) >> is compatible with the existing string_to_array( str, sep ), and >> "nullstr => 'NULL'" will be same as your to_array(). >> >> * array_to_string( arr anyarray, sep text, nullstr text DEFAULT NULL ) >> is compatible with the existing array_to_string(); separator also ignored >> when nullstr is NULL. "nullstr => ''" (an empty string) will be same as >> your to_array(). >> > > so reason for these new names are different default behave. And we > can't to change of default behave of existing functions. > > Regards > > Pavel Stehule > > > >> -- >> Itagaki Takahiro >> > -- 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: to_string, to_array functions
2010/7/12 Pavel Stehule : > I prefere a new names - because there are a new behave - with little > bit better default handling of NULL values. string_to_array and > array_to_string just ignore NULL values - what isn't correct behave. > Later we can mark these functions as deprecated and remove it. If I > use current function, then we have to continue in current behave. I prefer existing names because your new default behavior can be done with suitable nullstr values. IMHO, new names will be acceptable only if they are listed in the SQL-standard or many other databases use the names. Two similar versions of functions must confuse users. Also, are there any consensus about "existing functions are not correct" ? Since string_agg() and your new concat() functions ignores NULLs, I think it is not so bad for array_to_string() to ignore NULLs. -- Itagaki Takahiro -- 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: preload dictionary new version
2010/7/12 Tom Lane : > Itagaki Takahiro writes: >> 2010/7/8 Tom Lane : >>> For example, the dictionary-load code could automatically execute >>> the precompile step if it observed that the precompiled copy of the >>> dictionary was missing or had an older file timestamp than the source. I am not sure, but it can be recompiled when tseach code is actualised (minor update) too. > >> There might be a problem in automatic precompiler -- Where should we >> save the result? OS users of postgres servers don't have write-permission >> to $PGSHARE in normal cases. Instead, we can store the precompiled >> result to $PGDATA/pg_dict_cache or so. > > Yeah. Actually we'd *have* to do something like that because $PGSHARE > should contain only architecture-independent files, while the > precompiled files would presumably have dependencies on endianness etc. > It is file and can be broken - so we have to check its consistency. There have to some evidency of dictionaries in cache - how will be identified a precompiled file? We cannot use a dictionary name, because it is a combination of dictionary and stop words. Have to have to thinking about filenames length here? Will be beter some generated name and a new system table? Regards Pavel Stehule > 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
