pspp-dev
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[patch #6388] Support for reading postgres databases


From: Ben Pfaff
Subject: [patch #6388] Support for reading postgres databases
Date: Sun, 03 Feb 2008 07:32:10 +0000
User-agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.1.1) Gecko/20061205 Iceweasel/2.0.0.1 (Debian-2.0.0.1+dfsg-1)

Update of patch #6388 (project pspp):

                  Status:   Ready For Test/Review => Works For Me           

    _______________________________________________________

Follow-up Comment #1:

In the documentation, would it make sense to drop the FILE subcommand syntax
out of the generic GET DATA syntax summary, since it no longer applies to all
of the file types?

In at least one place "PostgresQL" is written in place of "PostgreSQL"
(missing capitalization on the "s").

Should "Postgres" be consistently capitalized in the documentation? 
Sometimes it is capitalized, sometimes it is not.

retreived -> retrieved

"Whether or not the connection is
encryption" 
->
"Whether or not the connection is
encrypted"

I am surprised that the best way to obtain the OID macros is to copy the set
of macros directly from the postgres source.  I assume that you considered
some alternatives, such as #include <catalog/pg_type.h>?

Is there something really subtle in the two implementations of
data_to_native()?  As far as I can tell, they do exactly the same job (copying
bytes and reversing them), but they write the output starting from opposite
ends.  I think that this only makes a difference if the input and output
buffers overlap.

The #ifndef USE_SSL/#define USE_SSL 1/#endif is curious.  It implies that
there's some code later on that depends on it, but I can't see any.

The //case_destroy (...) in psql_casereader_destroy() probably indicates some
missing code cleanup?

The following:
+         if ( var_is_numeric (v))
+           {
+             val->f = SYSMIS;
+           }
+         else
+           {
+             memset (val->s, ' ', var_get_width (v));
+           }
can be simplified to:
           value_set_missing (val, var_get_width (v));

I wonder whether the test should exit with status 77 if a postgres server
cannot be located?  The automake documentation says that tests that exit with
77 are ignored in the final count and that it should be used for nonportable
tests in environments where they don't make sense.

    _______________________________________________________

Reply to this item at:

  <http://savannah.gnu.org/patch/?6388>

_______________________________________________
  Message sent via/by Savannah
  http://savannah.gnu.org/





reply via email to

[Prev in Thread] Current Thread [Next in Thread]