Re: strange context message in spi.c?
The following review has been posted through the commitfest application: make installcheck-world: tested, failed Implements feature: tested, failed Spec compliant: tested, failed Documentation:tested, failed As tree is branched out for PG17, I guess now it's time to commit. - No need to rebase - make, make-check , install-check verified The new status of this patch is: Ready for Committer
Re: psql client does not handle WSAEWOULDBLOCK on Windows
I have not reproduce your test scenario, looking at code please see following comments: If you check the function definition of pqsecure_raw_read() it actually do set errno like bellow SOCK_ERRNO_SET(result_errno); where result_errno = SOCK_ERRNO Means anybody using those function pqsecure_raw_read/write, does not need to take care of portable ERRNO. Regards Umar Hayat The new status of this patch is: Waiting on Author
Re: ActiveState Perl is not valid anymore to build PG17 on the Windows 10/11 platforms, So Documentation still suggesting it should be updated
Hi Bruce, Patch looks good. In my opinion patch should be committed as more and more people who are trying to build PG 17 on Windows are facing similar issues. Latest issue was reported yesterday [0]. Regards Umar Hayat [0] https://www.postgresql.org/message-id/flat/CA%2BOCxozXN%3DGFPWU8vBjG6ch%3Di2q46FrQbTMjpsERGaSXQj-%2BtQ%40mail.gmail.com#8c5cefdb5881f4ed49e254d4ef9dbe09 Umar Hayat On Thu, 17 Oct 2024 at 07:35, Bruce Momjian wrote: > > On Tue, Jul 16, 2024 at 08:23:11AM -0400, Andrew Dunstan wrote: > > > > On 2024-07-16 Tu 7:46 AM, Yasir wrote: > > > > Hi Hackers, > > > > Recently, I compiled PG17 on the windows. Till PG16 "ActiveState Perl", > > as > > instructed in the documentation, was being used successfully on the > > Windows > > 10/11 to compile PG. > > However, it looks like that "ActiveState Perl" is not valid anymore to > > compile PG17 on Windows 10/11 but documentation still suggests it. > > Therefore, I think documentation needs to be updated. > > Moreover, I had to install "strawberry's perl" in order to compile PG17 > > on > > Windows 10/11. Please check out the thread "errors building on windows > > using meson" highlighting the issue. > > > > > > > > See https://postgr.es/m/4acddcd4-1c08-44e7-ba60-cab102259...@dunslane.net > > > > I agree we should fix the docco. > > I have written the attached patch to make these changes. > > -- > Bruce Momjian https://momjian.us > EDB https://enterprisedb.com > > When a patient asks the doctor, "Am I going to die?", he means > "Am I going to die soon?"
Re: msvc directory missing in PostgreSQL 17.0
Hi, In my opinion the issue could be Active-state perl ( as I see in your log file, activestate perl path). In this thread [0] people faced same problem and solved it using strawberry perl. Regards Umar Hayat [0] https://www.postgresql.org/message-id/flat/CADK3HHLQ1MNmfXqEvQi36D_MQrheOZPcXv2H3s6otMbSmfwjzg%40mail.gmail.com On Mon, 28 Oct 2024 at 10:31, 黄铎彦 wrote: > > On Oct 28, 2024 1:58 GMT+8, Andres Freund wrote: > >Without knowing the error it's hard for us to help... > > The build log is attatched. Note current environment on my Windows PC can > build pg 16.3 successfully, as I referenced > https://www.postgresql.org/docs/16/install-windows-full.html. > I didn't make any change to my dependencies. I use Windows 10 Home and Visual > Studio 2019.
Re: Add XMLNamespaces to XMLElement
Hi, +1 for the enhancement. I haven't compiled and reviewed the full patch yet, please see a few comments from my side based on static analysis. 1. Though changes are targeted for XMLNAMESPACES for XMLElement but in my opinion it will affect XMLTABLE as well because the 'xml_namespace_list' rule is shared now. Adding 'NO DEFAULT' in xml_namespace_list will allow users to use it with XMLTABLE XMLNAMESPACES as well.PostgreSQL grammar allow to specify DEFAULT in NAMESPACE but resulting in following error: "ERROR: DEFAULT namespace is not supported" What would be behavior with this change for XMLTABLE, should this be allowed and the error messages need to be updated (may be this will not be an error at all) or we need to restrict users to not use 'NO DEFAULT' with XMLTable. 2. Should we reuse the 'xml_namespaces' rule for XMLTable, as the definition is the same. 3. In this patch 'NO DEFAULT' behavior is like DEFAULT '' (empty uri) , should not it be more like 'DEFAULT NULL' to result in the following ? SELECT xmlelement(NAME "root", xmlnamespaces(NO DEFAULT)); xmlelement -- instead of SELECT xmlelement(NAME "root", xmlnamespaces(NO DEFAULT)); xmlelement -- Regards Umar Hayat On Sat, 21 Dec 2024 at 14:57, Pavel Stehule wrote: > > Hi > > so 21. 12. 2024 v 0:51 odesílatel Jim Jones > napsal: >> >> Hi, >> >> I'd like to propose the implementation of the XMLNamespaces option for >> XMLElement. >> >> XMLNAMESPACES(nsuri AS nsprefix) >> XMLNAMESPACES(DEFAULT default-nsuri) >> XMLNAMESPACES(NO DEFAULT) >> >> * nsprefix: Namespace's prefix. >> * nsuri: Namespace's URI. >> * DEFAULT default-nsuri: Specifies the DEFAULT namespace to use within >> the scope of a namespace declaration. >> * NO DEFAULT:Specifies that NO DEFAULT namespace is to be >> used within the scope of a namespace declaration. >> >> This basically works pretty much like XMLAttributes, but with a few more >> restrictions (see SQL/XML:2023, 11.2 ): >> >> * XML namespace declaration shall contain at most one DEFAULT namespace >> declaration item. >> * No namespace prefix shall be equivalent to xml or xmlns. >> * No namespace URI shall be identical to http://www.w3.org/2000/xmlns/ >> or to http://www.w3.org/XML/1998/namespace. >> * The value of a namespace URI contained in an regular namespace >> declaration item (no DEFAULT) shall not be a zero-length string. >> >> Examples: >> >> SELECT xmlelement(NAME "foo", xmlnamespaces('http://x.y' AS bar)); >> xmlelement >> --- >> http://x.y"/> >> >> SELECT xmlelement(NAME "foo", xmlnamespaces(DEFAULT 'http://x.y')); >> xmlelement >> --- >> http://x.y"/> >> >> SELECT xmlelement(NAME "foo", xmlnamespaces(NO DEFAULT)); >>xmlelement >> - >> >> >> In transformXmlExpr() it seemed convenient to use the same parameters to >> store the prefixes and URIs as in XMLAttributes (arg_names and >> named_args), but I am still not so sure it is the right approach. Is >> there perhaps a better way? >> >> Any thoughts? Feedback welcome! > > > +1 > > Pavel >> >> >> Best, Jim -- Umar Hayat Bitnine (https://bitnine.net/)
Re: Feature Request: Add AES-128-CFB Mode Support to pgcrypto
Hi Vladyslav, No Problem, I also did not realize that you will be implementing it. So I spent a couple of hours and provided a patch. Also created commitfest entry as well, Please do review the patch and let me know if this is sufficient at least for your use case. Regards, Umar Hayat Bitnine (https://bitnine.net/)
jsonlog missing from logging_collector description
Hi Hackers, Enabling logging_collector is required for json log. During jsonlog implementation [1], log destination and logging_collector comments in postgresql.conf.sample and documentation was updated but logging_collector short description was not updated in guc.c (which is guc_table.c now). Attached is the patch for this minor fix. [1] https://www.postgresql.org/message-id/CAH7T-aqswBM6JWe4pDehi1uOiufqe06DJWaU5=x7ddlyque...@mail.gmail.com Regards -- Umar Hayat Bitnine (https://bitnine.net/) v0-logging-collector-jsonlog.patch Description: Binary data
Add missing tab completion for VACUUM and ANALYZE with ONLY option
Hi, Recently the ONLY option is introduced [1] for VACUUM and ANALYZE commands. Attach provides improved tab completion for this new option for both commands. Along with this also added starting parenthesis "(" auto-complete as multiple existing commands provide it and it makes it easier to separate command options (VERBOSE, FREEZE, ... ) from table list. [1] https://www.postgresql.org/message-id/flat/CADofcAWATx_haD%3DQkSxHbnTsAe6%2Be0Aw8Eh4H8cXyogGvn_kOg%40mail.gmail.com Regards -- Umar Hayat Bitnine (https://bitnine.net/) v0-psql-vacuum-analyze-only-option.patch Description: Binary data
Re: jsonlog missing from logging_collector description
On Fri, 31 Jan 2025 at 20:41, Ashutosh Bapat wrote: > > On Fri, Jan 31, 2025 at 2:43 PM Michael Paquier wrote: > > > > On Fri, Jan 31, 2025 at 06:09:48PM +0900, Umar Hayat wrote: > > > Enabling logging_collector is required for json log. During jsonlog > > > implementation [1], log destination and logging_collector comments in > > > postgresql.conf.sample and documentation was updated but > > > logging_collector short description was not updated in guc.c (which is > > > guc_table.c now). > > > Attached is the patch for this minor fix. > > > > > > [1] > > > https://www.postgresql.org/message-id/CAH7T-aqswBM6JWe4pDehi1uOiufqe06DJWaU5=x7ddlyque...@mail.gmail.com > > > > Thanks for the report. This one is on me, will fix. > > -- > > Michael > > - gettext_noop("Start a subprocess to capture stderr output and/or > csvlogs into log files."), > + gettext_noop("Start a subprocess to capture stderr, csvlog and/or > jsonlog into log files."), > > Did you remove the word "output" intentionally? It is related to > stderr and not jsonlog. I think, stderr usually refers to a file or a > file descriptor hence we added word "output" there. I wouldn't touch > it. Yes, It was omitted intentionally to match the wording in postgresql.conf.sample and documentation of logging_collector. > > -- > Best Wishes, > Ashutosh Bapat -- Umar Hayat Bitnine (https://bitnine.net/)
Re: Add XMLNamespaces to XMLElement
Hi Jim & Pavel, Sorry for getting back a bit late on this. Few more case you might need consider: As I mentioned in my first static review about XMLTable existing behaviour might change, I give it a run time review and here are few findings: 1. Because of this patch XMLTable namespace will accept NO DEFAULT value, I was expecting an error message based on prior behavior '' but If I run following query it show something different: " > SELECT * FROM XMLTABLE(XMLNAMESPACES(NO DEFAULT), '/rows/row' PASSING 'http://x.y";>10' COLUMNS a int PATH 'a'); ERROR: cache lookup failed for type 0 " Can you please re-check this behaviour (there might be something wrong with my build environment) 2. Seems like namespace as ColumnRef was already allowed (I doubt if that's a correct implementation, I see other DB allows only strings AFAIK), for non-default namespaces may be its fair, should this be allowed for default namespace, any opinion. create table tbl1 ( col1 text); insert into tbl1 values ('abc'); insert into tbl1 values ('def'); select xmlelement(NAME "root", xmlnamespaces(default col1)) from tbl1; xmlelement ----- (2 rows) Changing status in commitfest, feel free to revert back. Regards Umar Hayat On Sun, 19 Jan 2025 at 18:33, Jim Jones wrote: > > > On 16.01.25 08:36, Pavel Stehule wrote: > > Now, I have not any objections against the code > > > > The patch has doc and enough regress tests > > The patching and compilation without problems > > make check-world passed > > > > I'll mark this patch as ready for committer > > > rebase due to gram.y changes introduced in 80feb72 > > > -- > Jim -- Umar Hayat Bitnine (https://bitnine.net/)
Re: Add XMLNamespaces to XMLElement
Pavel and Jim, If that's the case, it looks good to me. Just wanted to highlight potential issues and implementation differences compared to other databases. Regards Umar Hayat
Re: New process of getting changes into the commitfest app
On Mon, 27 Jan 2025 at 03:09, Yura Sokolov wrote: > > 23.01.2025 15:57, Jelte Fennema-Nio пишет: > > (Resent because sending to both -hackers and -www gets emails put in > > the moderation queue, and I don't want to introduce that delay to all > > replies. If you received the previous version because you're in the CC > > please only reply to this one) > > > > # Background > > > > As some of you might have noticed I've been trying to breathe some > > more life into development on the commitfest app[1], both by > > contributing myself but also by encouraging contributions of others. > > Basically I'd like to become one of the maintainers of the commitfest > > app project. The process to get there has been much more of a struggle > > than I'd hoped... > > > > ... > > > > I requested Magnus to give me commit access to the pgcommitfest repo > > so that I could deploy improvements without having to wait for his > > reviews. > > Given history of libxz backdoor, I'd fear to give "commit access" for > anything critical to rather fresh member of community. +1 in github you can enforce a minimum number of reviewers. IMO there should be a minimum of two reviewers and one of the reviewers should be from the security group/role. Though primary risk would be introducing new vulnerable dependency but there is no bound to other kinds of exploitation. Also github vulnerability scan should be enabled by default. > > I'm not in core-team though. > > -- Umar Hayat Bitnine (https://bitnine.net/)
Re: Add XMLNamespaces to XMLElement
On Tue, 21 Jan 2025 at 04:04, Jim Jones wrote: > > Hi Umar > > On 20.01.25 17:19, Umar Hayat wrote: > > Hi Jim & Pavel, > > Sorry for getting back a bit late on this. Few more case you might > > need consider: > > > > As I mentioned in my first static review about XMLTable existing > > behaviour might change, I give it a run time review and here are few > > findings: > > > > 1. Because of this patch XMLTable namespace will accept NO DEFAULT > > value, I was expecting an error message based on prior behavior '' but > > If I run following query it show something different: > > " > >> SELECT * FROM XMLTABLE(XMLNAMESPACES(NO DEFAULT), > > '/rows/row' > > PASSING ' > xmlns="http://x.y";>10' > > COLUMNS a int PATH 'a'); > > ERROR: cache lookup failed for type 0 > > " > > Can you please re-check this behaviour (there might be something wrong > > with my build environment) > > There is nothing wrong with your environment :) I forgot to test > XMLTable for NO DEFAULT namespaces. To be consistent with the existing > code, I added the following condition to transformRangeTableFunc: > > > if (!r->name && !r->val) > ns_uri = transformExpr(pstate, makeStringConst("", r->location), > EXPR_KIND_FROM_FUNCTION); > else > ns_uri = transformExpr(pstate, r->val, EXPR_KIND_FROM_FUNCTION); > > > It adds an empty string to the uri, which is the value expected for NO > DEFAULT. > > I also added a NO DEFAULT test for XMLTable in xml.sql > > v5 attached. > > > 2. Seems like namespace as ColumnRef was already allowed (I doubt if > > that's a correct implementation, I see other DB allows only strings > > AFAIK), for non-default namespaces may be its fair, should this be > > allowed for default namespace, any opinion. > > create table tbl1 ( col1 text); > > insert into tbl1 values ('abc'); > > insert into tbl1 values ('def'); > > select xmlelement(NAME "root", xmlnamespaces(default col1)) from tbl1; > > xmlelement > > - > > > > > > (2 rows) > > What are your concerns about supporting ColumnRef for the URI's? > > It is currently supported by XMLAttributes: > For XMLAttributes attribute it should have ColumnRef/Expr because that's the data/content we want to generate. But namespaces and xml tags, IMO they should be considered as part of the structure/schema of XML. Allowing namespaces (default or otherwise) to be generated arbitrarily for each record does not seem correct to me, it's like generating arbitrary XML using print string which does not require XML functions. - DB2 allows XMLElements namespace but it does not allow Expr/ColumnRef. - Oracle Allow namespace in only XMLTable, and it does not allow Expr/ColumnRef. - Having SConst/String or numeric can limit the error handling at parsing stage which can validate the schema instead of expression evaluation per record, which leads to following problem at runtime: CREATE TABLE xmltab (uri TEXT); INSERT INTO xmltab VALUES ('good'), (''); SELECT XMLElement(NAME "root", XMLNamespaces(uri AS zz)) from xmltab; ERROR: invalid XML namespace URI for "zz" DETAIL: a regular XML namespace cannot be a zero-length string Imagine there millions of records and in the middle it fails. - Also expression evaluation per record and doing validation (valid URI etc) I believe will have some performance impact ( I haven't tested it, so can't say for sure , how much ) - It's possible that there might be some other angle that I am missing right now, maybe Pavel/Àlvaro can shed light on this. I briefly went through the first implementation thread. There was a bit of discussion to use b_expr instead of c_expr, but I have yet to explore the reasoning of not using SConst. > CREATE TABLE t AS SELECT 'http://x.y' AS uri; > SELECT xmlelement(NAME el, xmlattributes("uri" AS att)) FROM t; > >xmlelement > > http://x.y"/> > (1 row) > > > Many thanks for the review! > > Best, Jim > > > > > -- > Jim -- Umar Hayat Bitnine (https://bitnine.net/)
Re: Feature Request: Add AES-128-CFB Mode Support to pgcrypto
Hi Daniel Gustafsson and Vladyslav Nebozhyn, I created a patch for CFB mode for AES encryption. Please have a look and let me know what you think. Patch covers implementation, tests and documentation changes. OpenSSL profives aes-cfb1, aes-cfb8 and aes-cfb128 modes where aes-cfb defaults to aes-cfb128. For simplicity I only added aes-cfb, which is the most common method used, lower number of bits will introduce performance degradation, but if it's desirable I can add them as well. -- Umar Hayat Bitnine (https://bitnine.net/) v0-0001-pgcrypto-aes-cfb-mode.patch Description: Binary data
Re: New committer: Jacob Champion
On Sat, 12 Apr 2025 at 05:26, Jonathan S. Katz wrote: > > The Core Team would like to extend our congratulations to Jacob > Champion, who has accepted an invitation to become our newest PostgreSQL > committer. > > Please join us in wishing Jacob much success and few reverts! > > Thanks, > > Jonathan Congratulations Jacob! -- Umar Hayat SKAI Worldwide (https://skaiworldwide.com/ <https://www.skaiworldwide.com/>)