Re: strange context message in spi.c?

2024-08-02 Thread Umar Hayat
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

2024-08-05 Thread Umar Hayat
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

2024-10-28 Thread Umar Hayat
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

2024-10-27 Thread Umar Hayat
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

2024-12-25 Thread Umar Hayat
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

2025-01-31 Thread Umar Hayat
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

2025-01-31 Thread Umar Hayat
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

2025-02-06 Thread Umar Hayat
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

2025-01-31 Thread Umar Hayat
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

2025-01-20 Thread Umar Hayat
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

2025-01-27 Thread Umar Hayat
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

2025-01-26 Thread Umar Hayat
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

2025-01-21 Thread Umar Hayat
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

2025-01-29 Thread Umar Hayat
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

2025-04-15 Thread Umar Hayat
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/>)