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