Re: [HACKERS] pg_upgrade code questions
Tom Lane wrote: > Magnus Hagander writes: >> On Thu, May 13, 2010 at 5:06 PM, Bruce Momjian wrote: >>> I have added SGML comments to comment out the text that mentions EDB >>> Advanced Server. Is that enough? Should I remove the text from the >>> SGML? Should I move it to the bottom of the SGML? Should I remove the >>> EnterpriseDB Advanced Server checks from the C code too? I don't >>> remember having to deal with anything like this before, so I am unclear >>> how to proceed. > >> I say remove it. On all accounts. > >> There's a fork of postgres for EDB AS, shouldn't there be a fork of >> pg_upgrade the same way, if it requires special code? The code in >> community postgresql certainly shouldn't have any EDB AS code in it. > > Indeed. Given the (presumably large) delta between EDB's code and ours, > having to have some delta in pg_upgrade isn't going to make much > difference for them. I think the community code and docs should > completely omit any mention of that. Speaking as the person who has been doing the EDB AS merges recently, I agree. It was helpful to have that stuff there when it was in pgfoundry, but now that it's part of the main repository, it just gets in the way. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_upgrade code questions
On Fri, May 14, 2010 at 5:34 AM, Bruce Momjian wrote: > Takahiro Itagaki wrote: >> >> Bruce Momjian wrote: >> >> > > >> 2. extern PGDLLIMPORT >> > > >> pg_upgrade has own definitions of >> > > >> extern PGDLLIMPORT Oid binary_upgrade_next_xxx >> > > >> > > > The issue here is that you use PGDLLIMPORT where you are importing the >> > > > variable, not where it is defined. For example, look at >> > > > 'seq_page_cost'. You can see PGDLLIMPORT used where it is imported >> > > > with >> > > > 'extern', but not where is it defined. >> > > >> > > Right. Also we are intentionally not exposing those variables in any >> > > backend .h file, because they are not meant for general use. So the >> > > "extern PGDLLIMPORT" isn't going to be in the main backend and has to >> > > be in pg_upgrade. This was discussed awhile ago when we put in those >> > > variables, I believe. >> > >> > Yes, this was discussed. >> >> I wonder some compilers or linkers might hide unexported global variables >> from postgres.lib as if they are declared with 'static' specifiers. >> I'm especially worried about Windows and MSVC. So, if Windows testers >> can see it works, there was nothing to worry about. > > Yes, none of the variables pg_upgrade is referencing are 'static', and > Magnus tested MSVC and checked MinGW compiles. Just to be clear, I only verified that it *built*, didn't have time to check if it actually *worked*. -- 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_upgrade code questions
On Thu, 2010-05-13 at 17:19 +0200, Magnus Hagander wrote: > I say remove it. On all accounts. > > There's a fork of postgres for EDB AS, shouldn't there be a fork of > pg_upgrade the same way, if it requires special code? The code in > community postgresql certainly shouldn't have any EDB AS code in it. Agreed. -- Devrim GÜNDÜZ PostgreSQL Danışmanı/Consultant, Red Hat Certified Engineer PostgreSQL RPM Repository: http://yum.pgrpms.org Community: devrim~PostgreSQL.org, devrim.gunduz~linux.org.tr http://www.gunduz.org Twitter: http://twitter.com/devrimgunduz signature.asc Description: This is a digitally signed message part
Re: [HACKERS] pg_upgrade code questions
Takahiro Itagaki wrote: > > Bruce Momjian wrote: > > > > >> 2. extern PGDLLIMPORT > > > >> pg_upgrade has own definitions of > > > >> extern PGDLLIMPORT Oid binary_upgrade_next_xxx > > > > > > > The issue here is that you use PGDLLIMPORT where you are importing the > > > > variable, not where it is defined. For example, look at > > > > 'seq_page_cost'. You can see PGDLLIMPORT used where it is imported with > > > > 'extern', but not where is it defined. > > > > > > Right. Also we are intentionally not exposing those variables in any > > > backend .h file, because they are not meant for general use. So the > > > "extern PGDLLIMPORT" isn't going to be in the main backend and has to > > > be in pg_upgrade. This was discussed awhile ago when we put in those > > > variables, I believe. > > > > Yes, this was discussed. > > I wonder some compilers or linkers might hide unexported global variables > from postgres.lib as if they are declared with 'static' specifiers. > I'm especially worried about Windows and MSVC. So, if Windows testers > can see it works, there was nothing to worry about. Yes, none of the variables pg_upgrade is referencing are 'static', and Magnus tested MSVC and checked MinGW compiles. -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_upgrade code questions
Bruce Momjian wrote: > > >> 2. extern PGDLLIMPORT > > >> pg_upgrade has own definitions of > > >> extern PGDLLIMPORT Oid binary_upgrade_next_xxx > > > > > The issue here is that you use PGDLLIMPORT where you are importing the > > > variable, not where it is defined. For example, look at > > > 'seq_page_cost'. You can see PGDLLIMPORT used where it is imported with > > > 'extern', but not where is it defined. > > > > Right. Also we are intentionally not exposing those variables in any > > backend .h file, because they are not meant for general use. So the > > "extern PGDLLIMPORT" isn't going to be in the main backend and has to > > be in pg_upgrade. This was discussed awhile ago when we put in those > > variables, I believe. > > Yes, this was discussed. I wonder some compilers or linkers might hide unexported global variables from postgres.lib as if they are declared with 'static' specifiers. I'm especially worried about Windows and MSVC. So, if Windows testers can see it works, there was nothing to worry about. Regards, --- Takahiro Itagaki 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] pg_upgrade code questions
Tom Lane wrote: > Bruce Momjian writes: > > Takahiro Itagaki wrote: > >> 2. extern PGDLLIMPORT > >> pg_upgrade has own definitions of > >> extern PGDLLIMPORT Oid binary_upgrade_next_xxx > >> in pg_upgrade_sysoids.c. But those variables are not declared as > >> PGDLLIMPORT in the core. Can we access unexported variables here? > > > The issue here is that you use PGDLLIMPORT where you are importing the > > variable, not where it is defined. For example, look at > > 'seq_page_cost'. You can see PGDLLIMPORT used where it is imported with > > 'extern', but not where is it defined. > > Right. Also we are intentionally not exposing those variables in any > backend .h file, because they are not meant for general use. So the > "extern PGDLLIMPORT" isn't going to be in the main backend and has to > be in pg_upgrade. This was discussed awhile ago when we put in those > variables, I believe. Yes, this was discussed. -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_upgrade code questions
Bruce Momjian writes: > Takahiro Itagaki wrote: >> 2. extern PGDLLIMPORT >> pg_upgrade has own definitions of >> extern PGDLLIMPORT Oid binary_upgrade_next_xxx >> in pg_upgrade_sysoids.c. But those variables are not declared as >> PGDLLIMPORT in the core. Can we access unexported variables here? > The issue here is that you use PGDLLIMPORT where you are importing the > variable, not where it is defined. For example, look at > 'seq_page_cost'. You can see PGDLLIMPORT used where it is imported with > 'extern', but not where is it defined. Right. Also we are intentionally not exposing those variables in any backend .h file, because they are not meant for general use. So the "extern PGDLLIMPORT" isn't going to be in the main backend and has to be in pg_upgrade. This was discussed awhile ago when we put in those variables, I believe. 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] pg_upgrade code questions
Takahiro Itagaki wrote: > I read pg_upgrade code glance over, and found 4 issues in it. > Are there any issues to be fixed before 9.0 release? > > 1. NAMEDATASIZE > 2. extern PGDLLIMPORT > 3. pathSeparator > 4. EDB_NATIVE_LANG > > 1. NAMEDATASIZE > pg_upgrade has the following definition, but should it be just NAMEDATALEN? > > /* Allocate for null byte */ > #define NAMEDATASIZE (NAMEDATALEN + 1) > > Table names should be in NAMEDATELEN - 1 bytes. At least 64th bytes in > "name" data is always '\0'. > > =# CREATE TABLE "1234567890...(total 70 chars)...1234567890" (i int); > NOTICE: identifier "123...890" will be truncated to "123...0123" Agreed. I have changed the code to use NAMEDATALEN. > 2. extern PGDLLIMPORT > pg_upgrade has own definitions of > extern PGDLLIMPORT Oid binary_upgrade_next_xxx > in pg_upgrade_sysoids.c. But those variables are not declared as > PGDLLIMPORT in the core. Can we access unexported variables here? The issue here is that you use PGDLLIMPORT where you are importing the variable, not where it is defined. For example, look at 'seq_page_cost'. You can see PGDLLIMPORT used where it is imported with 'extern', but not where is it defined. > 3. pathSeparator > Path separator for Windows is not only \ but also /. The current code > ignores /. Also, it might not work if the path string including multi-byte > characters that have \ (0x5c) in the second byte. Agreed. I have modified the code to use only "/" and check for "/" and "\". It is used only for checking the last byte so I didn't think it would affect a multi-byte sequence. I am actually unclear on that issue though. Can you review the new code to see if it is OK. > 4. EDB_NATIVE_LANG > Of course it is commented out with #ifdef, but do we have codes > for EDB in core? Yeah, removed. -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_upgrade code questions
Josh Berkus wrote:
> On 5/13/10 10:14 AM, Bruce Momjian wrote:
> > I am trying to think of this as a non-EnterpriseDB employee. If suppose
> > Greenplum had given us a utility and they wanted it to work with their
> > version of the database, what accommodation would we make for them? I
> > agree on the documentation, but would we allow #ifdefs that were only
> > used by them if there were only a few of them? Could we treat it as an
> > operating system that none of us use? I don't think Greenplum would
> > require us to keep support for their database, but they would prefer it,
> > and it might encourage more contributions from them. Maybe we would
> > just tell them to keep their own patches, but I figured I would ask
> > specifically so we have a policy for next time.
>
> My $0.021746:
>
> If something is going to be included in /contrib, it should only include
> code which relates to standard PostgreSQL. The independant pg_migrator
> project can be a PG/EDBAS tool; the contrib module needs to be
> vanilla-postgres only. If the donor of the code wants to keep the
> specific fork support, then it should remain an independant project.
>
> I'm not just referring to EDB here, or even just proprietary forks; even
> open source forks (like PostgresXC or pgCluster) shouldn't have specific
> code in /contrib. Within the limits of reasonableness, of course.
>
> My argument isn't based on purity, but is rather based on:
> (a) avoiding confusing the users, and
> (b) avoiding bulking code with lots of ifdefs if we can avoid it, and
> (c) fork release cycles are often different from pgsql-core, and EDB's
> certainly is.
I was more interested in understanding our policy rather than how to
handle this specific issue. I have removed all mentions of EnterpriseDB
Advanced Server from pg_upgrade with the attached patch. I will keep
the patch for submission back to EnterpriseDB when they want it, or they
can just pull it from CVS.
--
Bruce Momjian http://momjian.us
EnterpriseDB http://enterprisedb.com
Index: contrib/pg_upgrade/check.c
===
RCS file: /cvsroot/pgsql/contrib/pg_upgrade/check.c,v
retrieving revision 1.3
diff -c -c -r1.3 check.c
*** contrib/pg_upgrade/check.c 13 May 2010 15:58:15 - 1.3
--- contrib/pg_upgrade/check.c 13 May 2010 22:48:06 -
***
*** 149,158
{
prep_status(ctx, "Adjusting sequences");
exec_prog(ctx, true,
! SYSTEMQUOTE "\"%s/%s\" --set ON_ERROR_STOP=on --port %d "
"-f \"%s\" --dbname template1 >> \"%s\"" SYSTEMQUOTE,
! ctx->new.bindir, ctx->new.psql_exe, ctx->new.port,
! sequence_script_file_name, ctx->logfile);
unlink(sequence_script_file_name);
pg_free(sequence_script_file_name);
check_ok(ctx);
--- 149,158
{
prep_status(ctx, "Adjusting sequences");
exec_prog(ctx, true,
! SYSTEMQUOTE "\"%s/psql\" --set ON_ERROR_STOP=on --port %d "
"-f \"%s\" --dbname template1 >> \"%s\"" SYSTEMQUOTE,
! ctx->new.bindir, ctx->new.port, sequence_script_file_name,
! ctx->logfile);
unlink(sequence_script_file_name);
pg_free(sequence_script_file_name);
check_ok(ctx);
Index: contrib/pg_upgrade/controldata.c
===
RCS file: /cvsroot/pgsql/contrib/pg_upgrade/controldata.c,v
retrieving revision 1.1
diff -c -c -r1.1 controldata.c
*** contrib/pg_upgrade/controldata.c 12 May 2010 02:19:10 - 1.1
--- contrib/pg_upgrade/controldata.c 13 May 2010 22:48:06 -
***
*** 9,18
#include
#include
- #ifdef EDB_NATIVE_LANG
- #include "access/tuptoaster.h"
- #endif
-
/*
* get_control_data()
--- 9,14
***
*** 88,102
got_float8_pass_by_value = true;
}
- #ifdef EDB_NATIVE_LANG
- /* EDB AS 8.3 is an 8.2 code base */
- if (cluster->is_edb_as && GET_MAJOR_VERSION(cluster->major_version) <= 803)
- {
- cluster->controldata.toast = TOAST_MAX_CHUNK_SIZE;
- got_toast = true;
- }
- #endif
-
/* we have the result of cmd in "output". so parse it line by line now */
while (fgets(bufin, sizeof(bufin), output))
{
--- 84,89
***
*** 140,148
p++;/* removing ':' char */
cluster->controldata.cat_ver = (uint32) atol(p);
}
! else if ((p = strstr(bufin, "First log file ID after reset:")) != NULL ||
! (cluster->is_edb_as && GET_MAJOR_VERSION(cluster->major_version) <= 803 &&
! (p = strstr(bufin, "Current log file ID:")) != NULL))
{
p = strchr(p, ':');
--- 127,133
p++;/* removing ':' char */
cluster->controldata.cat_ver = (uint32) atol(p);
}
! else if ((p = strstr(bufin, "First log file ID after reset:")) != NULL)
{
p = strchr(p, ':');
***
*** 153,161
cluster->controldata.logid = (uint32) atol(p);
got_log_id = true;
}
! else i
Re: [HACKERS] pg_upgrade code questions
On 5/13/10 10:14 AM, Bruce Momjian wrote: > I am trying to think of this as a non-EnterpriseDB employee. If suppose > Greenplum had given us a utility and they wanted it to work with their > version of the database, what accommodation would we make for them? I > agree on the documentation, but would we allow #ifdefs that were only > used by them if there were only a few of them? Could we treat it as an > operating system that none of us use? I don't think Greenplum would > require us to keep support for their database, but they would prefer it, > and it might encourage more contributions from them. Maybe we would > just tell them to keep their own patches, but I figured I would ask > specifically so we have a policy for next time. My $0.021746: If something is going to be included in /contrib, it should only include code which relates to standard PostgreSQL. The independant pg_migrator project can be a PG/EDBAS tool; the contrib module needs to be vanilla-postgres only. If the donor of the code wants to keep the specific fork support, then it should remain an independant project. I'm not just referring to EDB here, or even just proprietary forks; even open source forks (like PostgresXC or pgCluster) shouldn't have specific code in /contrib. Within the limits of reasonableness, of course. My argument isn't based on purity, but is rather based on: (a) avoiding confusing the users, and (b) avoiding bulking code with lots of ifdefs if we can avoid it, and (c) fork release cycles are often different from pgsql-core, and EDB's certainly is. -- -- Josh Berkus PostgreSQL Experts Inc. http://www.pgexperts.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_upgrade code questions
Bruce Momjian wrote: Indeed. Given the (presumably large) delta between EDB's code and ours, having to have some delta in pg_upgrade isn't going to make much difference for them. I think the community code and docs should completely omit any mention of that. I am trying to think of this as a non-EnterpriseDB employee. If suppose Greenplum had given us a utility and they wanted it to work with their version of the database, what accommodation would we make for them? I agree on the documentation, but would we allow #ifdefs that were only used by them if there were only a few of them? Could we treat it as an operating system that none of us use? I don't think Greenplum would require us to keep support for their database, but they would prefer it, and it might encourage more contributions from them. Maybe we would just tell them to keep their own patches, but I figured I would ask specifically so we have a policy for next time. I guess another question is whether we would accept a patch that was useful only for a Greenplum build? And does removing such code use the same criteria? I know pgAdmin supports Greenplum, but that is an external tool so it makes more sense there. What if several vendors want the same thing? The code will quickly become spaghetti. AFAIK the Linux kernel expects distros to keep their patchsets separately, and I rather think we should too. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_upgrade code questions
On Thu, 2010-05-13 at 17:19 +0200, Magnus Hagander wrote: > I say remove it. On all accounts. > > There's a fork of postgres for EDB AS, shouldn't there be a fork of > pg_upgrade the same way, if it requires special code? The code in > community postgresql certainly shouldn't have any EDB AS code in it. If the code would be useful for other "projects" then keep it. If it is only for a closed source product, dump it. Joshua D. Drake -- PostgreSQL.org Major Contributor Command Prompt, Inc: http://www.commandprompt.com/ - 503.667.4564 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] pg_upgrade code questions
Tom Lane wrote: > Magnus Hagander writes: > > On Thu, May 13, 2010 at 5:06 PM, Bruce Momjian wrote: > >> I have added SGML comments to comment out the text that mentions EDB > >> Advanced Server. ?Is that enough? ?Should I remove the text from the > >> SGML? ?Should I move it to the bottom of the SGML? ?Should I remove the > >> EnterpriseDB Advanced Server checks from the C code too? ?I don't > >> remember having to deal with anything like this before, so I am unclear > >> how to proceed. > > > I say remove it. On all accounts. > > > There's a fork of postgres for EDB AS, shouldn't there be a fork of > > pg_upgrade the same way, if it requires special code? The code in > > community postgresql certainly shouldn't have any EDB AS code in it. > > Indeed. Given the (presumably large) delta between EDB's code and ours, > having to have some delta in pg_upgrade isn't going to make much > difference for them. I think the community code and docs should > completely omit any mention of that. I am trying to think of this as a non-EnterpriseDB employee. If suppose Greenplum had given us a utility and they wanted it to work with their version of the database, what accommodation would we make for them? I agree on the documentation, but would we allow #ifdefs that were only used by them if there were only a few of them? Could we treat it as an operating system that none of us use? I don't think Greenplum would require us to keep support for their database, but they would prefer it, and it might encourage more contributions from them. Maybe we would just tell them to keep their own patches, but I figured I would ask specifically so we have a policy for next time. I guess another question is whether we would accept a patch that was useful only for a Greenplum build? And does removing such code use the same criteria? I know pgAdmin supports Greenplum, but that is an external tool so it makes more sense there. -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_upgrade code questions
Magnus Hagander writes: > On Thu, May 13, 2010 at 5:06 PM, Bruce Momjian wrote: >> I have added SGML comments to comment out the text that mentions EDB >> Advanced Server. Is that enough? Should I remove the text from the >> SGML? Should I move it to the bottom of the SGML? Should I remove the >> EnterpriseDB Advanced Server checks from the C code too? I don't >> remember having to deal with anything like this before, so I am unclear >> how to proceed. > I say remove it. On all accounts. > There's a fork of postgres for EDB AS, shouldn't there be a fork of > pg_upgrade the same way, if it requires special code? The code in > community postgresql certainly shouldn't have any EDB AS code in it. Indeed. Given the (presumably large) delta between EDB's code and ours, having to have some delta in pg_upgrade isn't going to make much difference for them. I think the community code and docs should completely omit any mention of that. 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] pg_upgrade code questions
On Thu, May 13, 2010 at 5:06 PM, Bruce Momjian wrote: > Magnus Hagander wrote: >> On Thu, May 13, 2010 at 8:22 AM, Devrim G?ND?Z wrote: >> > On Thu, 2010-05-13 at 15:13 +0900, Takahiro Itagaki wrote: >> >> 4. EDB_NATIVE_LANG >> >> Of course it is commented out with #ifdef, but do we have codes >> >> for EDB in core? >> > >> > I was about to raise similar thing, for the documentation: >> > >> > http://developer.postgresql.org/pgdocs/postgres/pgupgrade.html >> > >> > This includes some references to EDB AS, which should be removed from >> > PostgreSQL official documentation, IMHO. >> >> +1 on getting rid of those references. > > Agreed. When it was on pgFoundry, I had to mention that because it was > unclear who would be using it, but in /contrib we know this is for > community Postgres. EnterpriseDB did contribute the code so I would > like to keep the code working for EnterpriseDB Advanced Server if that > is easy. > > I have added SGML comments to comment out the text that mentions EDB > Advanced Server. Is that enough? Should I remove the text from the > SGML? Should I move it to the bottom of the SGML? Should I remove the > EnterpriseDB Advanced Server checks from the C code too? I don't > remember having to deal with anything like this before, so I am unclear > how to proceed. I say remove it. On all accounts. There's a fork of postgres for EDB AS, shouldn't there be a fork of pg_upgrade the same way, if it requires special code? The code in community postgresql certainly shouldn't have any EDB AS code in it. -- 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_upgrade code questions
Magnus Hagander wrote: > On Thu, May 13, 2010 at 8:22 AM, Devrim G?ND?Z wrote: > > On Thu, 2010-05-13 at 15:13 +0900, Takahiro Itagaki wrote: > >> 4. EDB_NATIVE_LANG > >> Of course it is commented out with #ifdef, but do we have codes > >> for EDB in core? > > > > I was about to raise similar thing, for the documentation: > > > > http://developer.postgresql.org/pgdocs/postgres/pgupgrade.html > > > > This includes some references to EDB AS, which should be removed from > > PostgreSQL official documentation, IMHO. > > +1 on getting rid of those references. Agreed. When it was on pgFoundry, I had to mention that because it was unclear who would be using it, but in /contrib we know this is for community Postgres. EnterpriseDB did contribute the code so I would like to keep the code working for EnterpriseDB Advanced Server if that is easy. I have added SGML comments to comment out the text that mentions EDB Advanced Server. Is that enough? Should I remove the text from the SGML? Should I move it to the bottom of the SGML? Should I remove the EnterpriseDB Advanced Server checks from the C code too? I don't remember having to deal with anything like this before, so I am unclear how to proceed. -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_upgrade code questions
On Thu, May 13, 2010 at 8:22 AM, Devrim GÜNDÜZ wrote: > On Thu, 2010-05-13 at 15:13 +0900, Takahiro Itagaki wrote: >> 4. EDB_NATIVE_LANG >> Of course it is commented out with #ifdef, but do we have codes >> for EDB in core? > > I was about to raise similar thing, for the documentation: > > http://developer.postgresql.org/pgdocs/postgres/pgupgrade.html > > This includes some references to EDB AS, which should be removed from > PostgreSQL official documentation, IMHO. +1 on getting rid of those references. -- 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_upgrade code questions
On Thu, 2010-05-13 at 15:13 +0900, Takahiro Itagaki wrote: > 4. EDB_NATIVE_LANG > Of course it is commented out with #ifdef, but do we have codes > for EDB in core? I was about to raise similar thing, for the documentation: http://developer.postgresql.org/pgdocs/postgres/pgupgrade.html This includes some references to EDB AS, which should be removed from PostgreSQL official documentation, IMHO. Regards, -- Devrim GÜNDÜZ PostgreSQL Danışmanı/Consultant, Red Hat Certified Engineer PostgreSQL RPM Repository: http://yum.pgrpms.org Community: devrim~PostgreSQL.org, devrim.gunduz~linux.org.tr http://www.gunduz.org Twitter: http://twitter.com/devrimgunduz signature.asc Description: This is a digitally signed message part
[HACKERS] pg_upgrade code questions
I read pg_upgrade code glance over, and found 4 issues in it. Are there any issues to be fixed before 9.0 release? 1. NAMEDATASIZE 2. extern PGDLLIMPORT 3. pathSeparator 4. EDB_NATIVE_LANG 1. NAMEDATASIZE pg_upgrade has the following definition, but should it be just NAMEDATALEN? /* Allocate for null byte */ #define NAMEDATASIZE(NAMEDATALEN + 1) Table names should be in NAMEDATELEN - 1 bytes. At least 64th bytes in "name" data is always '\0'. =# CREATE TABLE "1234567890...(total 70 chars)...1234567890" (i int); NOTICE: identifier "123...890" will be truncated to "123...0123" 2. extern PGDLLIMPORT pg_upgrade has own definitions of extern PGDLLIMPORT Oid binary_upgrade_next_xxx in pg_upgrade_sysoids.c. But those variables are not declared as PGDLLIMPORT in the core. Can we access unexported variables here? 3. pathSeparator Path separator for Windows is not only \ but also /. The current code ignores /. Also, it might not work if the path string including multi-byte characters that have \ (0x5c) in the second byte. 4. EDB_NATIVE_LANG Of course it is commented out with #ifdef, but do we have codes for EDB in core? Regards, --- Takahiro Itagaki 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
