|
Bug Notes |
|
|
|
|
Yes, I can imagine problems, when joining column is not a unique/primary key. However we have to consider if this is really a bug. Bad joining is requested by user, phpMyEdit just does its job... |
|
|
|
|
since it does not behave the way i expect, i consider it a bug :) btw what's definition of bug ?
anyway, i can define this to be a feature request to have
'output_filter' => '$alias.col LIKE abs'
in values definition which would result in filtering all possible values. sounds reasonable ? |
|
|
|
hbernard
|
|
2003-11-14 07:20
|
|
the ['values']['filter'] clause in not used during LIST/FILTER joining.
This is convenient if you want to restrict input to a set of values but want to allow to see correctly all the records in List/Filter mode.
Changing this will broke my apps.
But there is no way to handle correctly michal sample without :
-1- his patch
-2- another 'values'/options
His patch is consistent (if the options 'filter' is set, use it).
Then, if user don't want of it in List/filter mode you'll have to play with
'value|LF' => ............
and
'values|ACPDV' => .......... |
|
|
|
|
after some usage, i realized that the patch is not correct:
1. it's not backward compatible
2. it fails if filter is 'col1 = 2 AND col2=7'
that's why i propose a separate filter, which would be evaled using $alias and thus (probably) working
also for values|F i would expect possibility like
'date >= 2003-01-01 AND date <= 2003-01-31' => 'jan 2003'
but that's a long and different story ... it's started here http://platon.sk/forum/projects/viewtopic.php?t=204
i forget: values == values|ACPDF (from usage of | in help,...) and therefore it would also break old applications
edited on: 2003/11/14 20:03 |
|
|
|
|
i uploaded another version of the join patch.
i had to add a new option
$opts['fdd']['col_name']['values'] = '$alias.col > 3 AND $alias.col2 = 4';
or in my case
$opts['fdd']['col_name']['values'] = '$alias.to_col_name = '.$col_name;
the background: i have a pme form where i want users to choose from a dropdown menu and i want an admin to add some options whene needed. i used values2 option, but then the admin had to add something to a text file located on the server. so i created a table with options (three columns: to_col_name, id_2, description; to_col_name with id_2 primary key; to_col_name says which column from the main/first table is referenced to ). since id_2 does not have to be unique, i ran into this problem. |
|
|
|
|
i uploaded a diff against current v 1.112 (with no eval).
into documentation:
Sometimes you want to restrict table joining on the output. This is important in case where ['values']['column'] is not unique in ['values']['table']. ['values']['filters'] will filter this on input and ['values']['output_filters'] will filter this on the output. Note that usage of $alias in front of every column name is important.
basic output filters usage:
$opts['fdd']['col_name']['values']['output_filters'] = '$alias.col > 3 AND $alias.col2 = 4';
or
$opts['fdd']['col_name']['values']['output_filters'] = '$alias.which_group = 4'; |
|
|
|
hbernard
|
|
2004-01-11 13:20
|
|
Well, can we add a more generic option for handling this ?
I mean, sometimes, I would need to be able to totally control the left join.
Your patch is fine, but the join is still partially hardcoded in the main class.
API proposal :
$opts['fdd']['col_name']['values']['join'] = '$alias.column_ID = PMEtable0.col_name AND $alias.col > 3 AND $alias.col2 = 4';
[if 'join' is set, the 'column' options has no effect]
Then the patch will be something like :
Ln 532 :
if (!in_array($alias,$tbs)) {
$join .= " LEFT OUTER JOIN $db.$table AS $alias";
- $join .= " ON $alias.$id = PMEtable0.$field";
+ $join .= " ON ";
+ $join .= isset($this->fdd[$field]['values']['join'])
+ ? $this->substituteVars($this->fdd[$field]['values']['join'], array('alias' => $alias))
+ : "$alias.$id = PMEtable0.$field";
$tbs[]=$alias;
}
}
edited on: 2004-01-11 14:43 |
|
|
|
|
ok, it's fine with me.
separate joins for AC and LFV can be done by values|ac (i hope)
and PMEtable0 - won't this be changed by bug on prefixes ?
and i forgot: add $alias, $id, $desc, $field to possible variables in join opts. i don't understand what this really does, but i guess they are needed :)
edited on: 2004-01-11 18:05 |
|
|
|
|
to get straght the api. there are several variables:
$alias - name of the joining table
$id - id field in that table
$desc - ?? (where is this taking effect ?)
PMEtable0 - name of the main table
$field - field in this table
those names are really ... misleading
what names would you suggest ? hbernard, you seem to have good ideas for names. |
|
|
|
hbernard
|
|
2004-01-13 15:23
|
|
I'm unsure to have good ideas for names (see the 'notebook' initial name for tabs ;-)
the only variables (I think) that are required are:
$alias, and I would rename it $tbjoinalias
$tbmainalias for PMEtable0
$id, $desc, $field are useless (if you're writing the 'join', I hope you know the fields that you want to use....)
BTW, there is another case where I need to be able to curomize the query.
This is not when joining to the main table, but when retrieving possible user input values. Did you encounter this case ? |
|
|
|
|
i vote for $tbjoin, $tbmain (shorter) and $idjoin, $idmain for $id and $field.
for $id-s: if you have cetralized definition of colums, like $opts['fdd']['col_name'] = my_column('Column name'); you don't know field name :) i know, there are workarounds, but why ?
'filters' option servers me well, so far. my sql syntax knowledge is not that high, so far.... |
|
|
|
hbernard
|
|
2004-01-13 16:12
|
|
About aliases : OK, using ${} syntax maybe (see your bug report about substitution) but this add again some string replacement (I think we really will have to make some code profiling).
${tbmain} should be available in the global $opts['filters'],
and maybe having ${tbjoin["fieldname"]} avalaible too. |
|
|
|
|
what would ${tbjoin["fieldname"]} be ?
$opts['filters'] - good comment.
'join' option: that is everything after 'ON', so default is '$tbjoin.$idjoin = $tbmain.$idmain' . and in my case '$tbjoin.$idjoin = $tbmain.$idmain AND $tbjoin.`group_name` = 4' . |
|
|
|
hbernard
|
|
2004-01-13 16:42
|
|
OK for default join declaration.
${tbjoin["fieldname"]} is for the $opts['filters'] stuff. This is the alias for the joined table to the field.
An example :
I have currently in some apps:
$opts['filters'] = "PMEjoin15.label NOT LIKE '%CLOS%'";
(because label is a field name in multiple join tables, I can't just use label)
And I want to be able to have :
$opts['filters'] = "${tbjoin["states"]}.label NOT LIKE '%CLOS%'";
('states' is the 15th field in my pme script, and there is a join with 'values']['table'] defined there. |
|
|
|
|
ok, good example.
i uploaded some diff, to v112.
looks working ?
btw, won't we start the sql quote (``) issue ? main class is not that of a problem, but user defined filters/joins....
edited on: 2004-01-13 17:00 |
|
|
|
hbernard
|
|
2004-01-13 17:06
|
|
looks good (but is reverted, isn't it ?)
But if we want to introduce $tbmain etc... in $opts['filters'], then maybe string replacement has to happen in the myquery level. |
|
|
|
|
sorry :)
yes but then dilema $tbjoin >< $tbjoin[field_name] and $idjoin, $idmain.
so i propose $join[field_name] so you can use also in the JOIN clause (can you imagine usefullness of this ? ) and make string replacement on both levels. works with my version of substituteVars. |
|
|
|
hbernard
|
|
2004-01-13 21:09
|
|
about the ambiguous name $tbjoin vs. $tbjoin[] : ok for $join["field_name"].
And another thing we have to take care about :
The select for list/change/Copy/delete/view are fully aliased, with PMEtable0 and so on.
But for different reason, the select for input dropdown, and for delete/update/insert are not (because for example, you can't use aliases when updating in MySQL).
I went with this in my first version of SQL-aliasing (August, see sqlw bug report).
This can be a concern when an option apply for both type of querys (aliased or not). |
|
|
|
|
do we agree on the above patch to be added ?
so you want 'join' clause available also for dropdown boxes (?) and replace 'filters'. inserting/updating is done purely by POST data (afaik), no joining in that case.
i think $tbjoin=$tbmain;$idjoin=$idmain; will allow this (since 1=1)(if you use them, problem if they are hardcoded).
in which cases current 'filters' option is not enough ? |
|
|
|
hbernard
|
|
2004-01-13 22:03
|
|
well, we agree but let's wait for nepto. Maybe the $variable is not a good syntax...
For the other case about 'filters', this is another story (will post an example). |
|
|
|
|
join-version5.diff againts v112 (not-reverted version of version4)
summary:
API ['values']['join'] = '...'; is part of join, everything after 'ON'
possible arguments:
$tbmain - alias for main table (currently PMEtable0);
$tbjoin - alias for joining table (currently PMEjoin4)
$idmain - field in $tbmain which is joined
$idjoin - field in $tbjoin (=['values']['column'])
and default value is "$tbjoin.$idjoin = $tbmain.$idmain"
this option is only valid for output. |
|
|
|
|
OK, I believe this is ready for commit. It seems to be a backward compatible.
I will suggest names $join_table, $join_id, $main_table, $main_id, however this really not a issue.
The problem is what we will made available in $this->filters ($opts['filters']).
$main_table and $main_id are really not a problem.
Particular join tables are problem. And we really do not want to have substituteVars() substitute a array-like variables.
Thus I suggest something like $join_table_colname
For example for $opts['fdd']['author_id']['values']['table']='authors' and in filters we can made $join_table_author_id available which will be substitued for "authors".
Problem can be with column names containing special characters, such as "-", "?", etc... we can end up with $this-strange?varaible - but substituteVars() is not a PHP parser, thus it may work. :)
Just a rough thoughts. Further notes are welcomed. :) |
|
|
|
|
Btw. better names:
$join_table
$join_column
$join_description
according to ['values'] key names |
|
|
|
|
so variable names could be $join_table, $join_column, $join_description, $main_table, $main_column.
let's make $opts['filters'] substitution separate. after substituteVars vs. array is solved. |
|
|
|
|
Patch was commited into CVS. New mentioned variables was introduced into ['join'].
Now we have to write documentation.
$opts['filters'] modification related to this should be opened as a new bug-report. |
|
|
|
|
I uploaded a documentation.
Please check if:
1. feature desription is correct;
2. example is good;
3. grammar is OK.
Thanks. |
|
|
|
hbernard
|
|
2004-04-04 21:45
|
|
1. OK
Can we add this line just after feature description :
Using the ['values']['filters'] simply will not work because it is not applied at join time, but only when filling values in the drop down.
2. OK
3. Doug, your comments ? |
|
|
|
|
Text added. Thanks.
Doug already verified my texts and I believe your sentence is correct, so no need for spell-checking. :) |
|
|
|
hbernard
|
|
2004-04-12 12:14
|
|
uploaded customjoin.diff.
This corrects a typo-cut-and-paste bug that prevented this feature to work.
Please Commit. |
|
|
|
|
Commited. Thanks!
Do we leave this report open till the $this->filters JOIN variables won't be open or we will close it and open new one for $this->filters JOIN variables? |
|
|
|
hbernard
|
|
2004-04-13 12:12
|
|
filters and current bug are strongly related.
I suggest to keep this one open and handle filters stuff here |
|
|
|
hbernard
|
|
2004-12-05 20:32
|
|
uploaded filters-v1.patch against v1.154
this is the implementation of variables support for $opts['filters'] option.
1. API :
variables available are :
$joined_foo_table, $joined_foo_column, $joined_foo_description, $main_table
where foo is the name where the join is set.
2. Patch :
some code has been put inside a join_array() method, in order avoir code duplication,
nothing really interesting.
Comments welcome, as usual. |
|
|
|
hbernard
|
|
2005-01-03 07:03
|
|
Nepto,
* if you agree on my ideas about values2 stuff in #296, this bug has to be handled _after_ API wrapping, adding leftjoin API, etc...
* if not, what do you think of this ? ;-) |
|