Re: [HACKERS] pg_upgrade code questions

2010-05-15 Thread Heikki Linnakangas
Tom Lane wrote:
 Magnus Hagander mag...@hagander.net writes:
 On Thu, May 13, 2010 at 5:06 PM, Bruce Momjian br...@momjian.us 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

2010-05-14 Thread Devrim GÜNDÜZ
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

2010-05-14 Thread Magnus Hagander
On Fri, May 14, 2010 at 5:34 AM, Bruce Momjian br...@momjian.us wrote:
 Takahiro Itagaki wrote:

 Bruce Momjian br...@momjian.us 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

2010-05-13 Thread Devrim GÜNDÜZ
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


Re: [HACKERS] pg_upgrade code questions

2010-05-13 Thread Magnus Hagander
On Thu, May 13, 2010 at 8:22 AM, Devrim GÜNDÜZ dev...@gunduz.org 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

2010-05-13 Thread Bruce Momjian
Magnus Hagander wrote:
 On Thu, May 13, 2010 at 8:22 AM, Devrim G?ND?Z dev...@gunduz.org 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  br...@momjian.ushttp://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

2010-05-13 Thread Magnus Hagander
On Thu, May 13, 2010 at 5:06 PM, Bruce Momjian br...@momjian.us wrote:
 Magnus Hagander wrote:
 On Thu, May 13, 2010 at 8:22 AM, Devrim G?ND?Z dev...@gunduz.org 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

2010-05-13 Thread Tom Lane
Magnus Hagander mag...@hagander.net writes:
 On Thu, May 13, 2010 at 5:06 PM, Bruce Momjian br...@momjian.us 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

2010-05-13 Thread Bruce Momjian
Tom Lane wrote:
 Magnus Hagander mag...@hagander.net writes:
  On Thu, May 13, 2010 at 5:06 PM, Bruce Momjian br...@momjian.us 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  br...@momjian.ushttp://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

2010-05-13 Thread Joshua D. Drake
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

2010-05-13 Thread Andrew Dunstan



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

2010-05-13 Thread Josh Berkus
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

2010-05-13 Thread Bruce Momjian
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  br...@momjian.ushttp://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 ctype.h
  #include stdlib.h
  
- #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 if ((p = strstr(bufin, First log file segment after reset:)) != NULL 

Re: [HACKERS] pg_upgrade code questions

2010-05-13 Thread Bruce Momjian
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  br...@momjian.ushttp://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

2010-05-13 Thread Tom Lane
Bruce Momjian br...@momjian.us 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

2010-05-13 Thread Bruce Momjian
Tom Lane wrote:
 Bruce Momjian br...@momjian.us 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  br...@momjian.ushttp://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

2010-05-13 Thread Takahiro Itagaki

Bruce Momjian br...@momjian.us 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

2010-05-13 Thread Bruce Momjian
Takahiro Itagaki wrote:
 
 Bruce Momjian br...@momjian.us 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  br...@momjian.ushttp://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