Re: [HACKERS] splitting plpython into smaller parts
On 18/12/11 20:53, Peter Eisentraut wrote:
> On tis, 2011-12-06 at 00:58 +0100, Jan Urbański wrote:
>> Rebased against master after the SPI cursor patch has been committed.
>>
>> The first patch removes SPI boilerplate from the cursor functions as
>> well and the second patch creates a plpython_cursor.c file.
>>
>> A side effect of creating a separate file for cursors is that I had to
>> make PLy_spi_transaction_{begin,commit,abort} helper functions external
>> since they're used both by regular SPI execution functions and the
>> cursor functions.
>>
>> They live the plpython_spi.c which is not an ideal place for them, but
>> IMHO it's not bad either.
>
> Committed now.
Great, thanks! I hope this will make for a more maintanable PL/Python.
By the way, the buildfarm is turning red because it's missing the
attached patch.
Cheers,
Jan
diff --git a/src/pl/plpython/Makefile b/src/pl/plpython/Makefile
index 12ce26e..a31328d 100644
*** a/src/pl/plpython/Makefile
--- b/src/pl/plpython/Makefile
*** endif # can't build
*** 189,195
# distprep and maintainer-clean rules should be run even if we can't build.
# Force this dependency to be known even without dependency info built:
! plpython_plpy.o: spiexceptions.h
spiexceptions.h: $(top_srcdir)/src/backend/utils/errcodes.txt generate-spiexceptions.pl
$(PERL) $(srcdir)/generate-spiexceptions.pl $< > $@
--- 189,195
# distprep and maintainer-clean rules should be run even if we can't build.
# Force this dependency to be known even without dependency info built:
! plpy_plpymodule.o: spiexceptions.h
spiexceptions.h: $(top_srcdir)/src/backend/utils/errcodes.txt generate-spiexceptions.pl
$(PERL) $(srcdir)/generate-spiexceptions.pl $< > $@
--
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] splitting plpython into smaller parts
On tis, 2011-12-06 at 00:58 +0100, Jan Urbański wrote:
> Rebased against master after the SPI cursor patch has been committed.
>
> The first patch removes SPI boilerplate from the cursor functions as
> well and the second patch creates a plpython_cursor.c file.
>
> A side effect of creating a separate file for cursors is that I had to
> make PLy_spi_transaction_{begin,commit,abort} helper functions external
> since they're used both by regular SPI execution functions and the
> cursor functions.
>
> They live the plpython_spi.c which is not an ideal place for them, but
> IMHO it's not bad either.
Committed now.
I moved a few more things around. I split up the plpython.h header file
to create a separate header file for each .c file, so that the
hierarchical releationship of the modules is clearer. (The only cases
of circular relationships should be caused by use of global variables.)
Also, I named the files that contain classes or modules more like they
are in the CPython source code, e.g., plpy_cursorobject.c.
--
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] splitting plpython into smaller parts
Alvaro Herrera writes: > Excerpts from Peter Eisentraut's message of jue dic 15 12:00:13 -0300 2011: >> How to people feel about naming the files (as proposed) >> >> ! OBJS = plpython.o plpython_io.o plpython_procedure.o plpython_exec.o \ >> !plpython_plpy.o plpython_spi.o plpython_result.o plpython_cursor.o \ >> !plpython_plan.o plpython_subtransaction.o plpython_functions.o \ >> !plpython_elog.o >> >> vs. say >> >> ! OBJS = main.o io.o procedure.o exec.o plpy.o spi.o result.o cursor.o \ >> !plan.o subtransaction.o functions.o elog.o >> >> ? > I find the extra prefix unnecessary and ugly; if we had to had a > prefix, I'd choose a shorter one (maybe "py" instead of "plpython_"). +1 for a prefix, mainly because the shorter names duplicate some names already in use elsewhere in our tree. But I agree with Alvaro that "py" would be sufficient. 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] splitting plpython into smaller parts
Excerpts from Peter Eisentraut's message of jue dic 15 12:00:13 -0300 2011: > How to people feel about naming the files (as proposed) > > ! OBJS = plpython.o plpython_io.o plpython_procedure.o plpython_exec.o \ > !plpython_plpy.o plpython_spi.o plpython_result.o plpython_cursor.o \ > !plpython_plan.o plpython_subtransaction.o plpython_functions.o \ > !plpython_elog.o > > vs. say > > ! OBJS = main.o io.o procedure.o exec.o plpy.o spi.o result.o cursor.o \ > !plan.o subtransaction.o functions.o elog.o > > ? I find the extra prefix unnecessary and ugly; if we had to had a prefix, I'd choose a shorter one (maybe "py" instead of "plpython_"). -- Álvaro Herrera The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- 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] splitting plpython into smaller parts
How to people feel about naming the files (as proposed) ! OBJS = plpython.o plpython_io.o plpython_procedure.o plpython_exec.o \ !plpython_plpy.o plpython_spi.o plpython_result.o plpython_cursor.o \ !plpython_plan.o plpython_subtransaction.o plpython_functions.o \ !plpython_elog.o vs. say ! OBJS = main.o io.o procedure.o exec.o plpy.o spi.o result.o cursor.o \ !plan.o subtransaction.o functions.o elog.o ? -- 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] splitting plpython into smaller parts
On mån, 2011-11-28 at 02:00 -0800, Greg Smith wrote: > On 11/13/2011 09:45 AM, Jan Urbański wrote: > > The first one factors out some bolerplate related to executing SPI > > functions in subtransactions (and idea borrowed from pltcl.c). > > While I haven't looked at the code, this seems worthwhile from the > description. > > > The second one is the actual split. plpython.c has been split into 11 > > separate files and one header. > > Could you comment a bit more about what the goal of this is? We don't > have a reviewer for this patch yet, and I think part of the reason is > because it's not really obvious what it's supposed to be doing, and why > that's useful. I will look into it. -- 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] splitting plpython into smaller parts
On 28/11/11 11:00, Greg Smith wrote: On 11/13/2011 09:45 AM, Jan Urbański wrote: The second one is the actual split. plpython.c has been split into 11 separate files and one header. Could you comment a bit more about what the goal of this is? We don't have a reviewer for this patch yet, and I think part of the reason is because it's not really obvious what it's supposed to be doing, and why that's useful. The idea of splitting plpython.c (an almost 5k lines file) into something more modular. It's been floated around here: http://postgresql.1045698.n5.nabble.com/Large-C-files-tt4766446.html#a4773493 and I think at other occasions, too. The patch introduces no behavioural changes, it's only shuffling code around. The only goal is to improve the maintainability. I guess the reviewer could verify that a) I haven't botched the split and it all still compiles and workds b) the choice of which modules were defined is correct Cheers, Jan -- 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] splitting plpython into smaller parts
On 11/13/2011 09:45 AM, Jan Urbański wrote: The first one factors out some bolerplate related to executing SPI functions in subtransactions (and idea borrowed from pltcl.c). While I haven't looked at the code, this seems worthwhile from the description. The second one is the actual split. plpython.c has been split into 11 separate files and one header. Could you comment a bit more about what the goal of this is? We don't have a reviewer for this patch yet, and I think part of the reason is because it's not really obvious what it's supposed to be doing, and why that's useful. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
