Hi Jan,
your code looks quite good.
Picky as I am I however have some comments :)
Jan
On 04/10/2008 03:38 PM, Jan Flokstra wrote with possible deletions:
> [...]
> U builtins.c
> Index: builtins.c
> ===================================================================
> RCS file: /cvsroot/monetdb/pathfinder/compiler/algebra/builtins.c,v
> retrieving revision 1.84
> retrieving revision 1.85
> diff -u -d -r1.84 -r1.85
> --- builtins.c 3 Apr 2008 09:42:11 -0000 1.84
> +++ builtins.c 10 Apr 2008 13:38:38 -0000 1.85
> @@ -44,6 +44,7 @@
> #include <assert.h>
> #include <stdio.h>
>
> +#include "mem.h"
^^^^^
what do you need that for?
> +
> +/*
> + * PFTIJAH defines. I decided not make a seperate include file for
> + * pftijah so ensure that these defines are exactly the same as in
> + * ../mil/milgen.brg
> + */
that is ugly in my eyes! -- You could place your stuff in algebra.h like
e.g., the axis steps instead.
> +
> +#define MYNODEKIND aat_pnode
> +#define DOCMGMTTYPE aat_docmgmt
> +
> +#define PFT_FUN(F) (strncmp(F,"pftijah_",8)==0)
> +
> +#define PFT_QUERY_N_XX "pftijah_query_n_xx"
> +#define PFT_QUERY_N_SX "pftijah_query_n_sx"
> +#define PFT_QUERY_N_XO "pftijah_query_n_xo"
> +#define PFT_QUERY_N_SO "pftijah_query_n_so"
> +#define PFT_QUERY_I_XX "pftijah_query_i_xx"
> +#define PFT_QUERY_I_SX "pftijah_query_i_sx"
> +#define PFT_QUERY_I_XO "pftijah_query_i_xo"
> +#define PFT_QUERY_I_SO "pftijah_query_i_so"
> +
> +#define PTF_QUERY_NODES(N) (N[14]=='n')
> +#define PTF_QUERY_STARTNODES(N) (N[16]=='s')
> +#define PTF_QUERY_OPTIONS(N) (N[17]=='o')
Encoding all information in a single string is even more ugly!!!
You have a context (ctx) where you can store arbitrary information. Just
build your own struct (in algebra.h) and make it less error-prone.
> +
> +PFalg_schema_t pft_empty_schema(PFalg_simple_type_t item_t) {
> + PFalg_schema_t schema;
> + schema.count = 3;
> + // INCOMPLETE, what about freeing this space
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
we use garbage collection.
> +struct PFla_pair_t pft_query_param2(struct PFla_pair_t *p1,
> PFalg_simple_type_t itemType1, struct PFla_pair_t *p2, PFalg_simple_type_t
> itemType2) {
> [...]
> +struct PFla_pair_t pft_query_param3(struct PFla_pair_t *p1,
> PFalg_simple_type_t itemType1, struct PFla_pair_t *p2, PFalg_simple_type_t
> itemType2, struct PFla_pair_t *p3, PFalg_simple_type_t itemType3) {
textwidth > 80
--
Jan Rittinger
Database Systems
Technische Universität München (Germany)
http://www-db.in.tum.de/~rittinge/
-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference
Don't miss this year's exciting event. There's still time to save $100.
Use priority code J8TL2D2.
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone
_______________________________________________
Monetdb-developers mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/monetdb-developers
