|
Rozšírené detaily chyby |
[ Preskočiť na poznámky k chybám ]
[ História chýb ]
[ Vytlačiť ]
|
|
ID |
Kategória |
Dôležitosť |
Reprodukovateľnosť |
Dátum vloženia |
Posledná zmena |
|
0000121 |
core / field option |
vlastnosť |
vždy |
2003-07-29 12:06 |
2006-01-23 01:46 |
|
|
|
Autor |
hbernard |
Zobraziť status |
verejný |
|
|
Priradené |
nepto |
|
Priorita |
normálna |
Riešenie |
vyriešený |
Platforma |
PC |
|
Stav |
uzavretý |
Duplicitné ID |
|
OS |
(Server) Linux Mandrake 9.1 |
|
Projection |
malá zmena |
|
Verzia |
Builtin |
|
ETA |
žiadny |
|
Verzia produktu |
current from CVS |
|
|
Build produktu |
|
|
|
Hlasy |
1 |
|
|
|
Zhrnutie |
Is 'sql' option only for _display_ useful |
|
Popis |
Using 'sql' option like this :
$opts["fdd"]["ID"]["sql"] = "concat(ID1,'-',ID2)";
Is there is a way to set the value during insert/update operation.
(I mean _after_ clicking save/insert button)
For example - If I fill in the fields :
ID1 =3
ID2 =4
I would like to see the ID field automagically set to '3-4'. |
|
Kroky k vyvolaniu |
|
|
Dalšie informácie |
This behaviour should happen only in Readonly fields |
|
|
|
Popis systému |
|
|
Pripojené súbory |
phpMyEdit.class.php (102,573 bytes) 2003-08-18 18:27
know.sql (1,398 bytes) 2003-08-18 18:27
know.php (5,854 bytes) 2003-08-18 18:28
diff.txt (17,670 bytes) 2003-10-07 11:43
commented - diff.txt (18,812 bytes) 2003-10-07 11:43
hb-sqlw-recreate.diff (1,591 bytes) 2004-01-21 22:15
hb-sqlw-recreate2.diff (3,856 bytes) 2004-01-22 10:22
cancelcopy.diff (561 bytes) 2004-04-17 12:23 |
|
|
|
Poznámky k chybe |
hbernard
2003-07-29 12:23
|
FYI : I know this could be set in triggers (no ?), but if this feature was available, it could be a workaround to deal with multiple keys. |
|
|
nepto
2003-08-16 15:59
|
Currently the experimental ['sql'] options applies only to table list and filtering pages. This is going to change to apply for all pages. Thus for example on the only record change page you can have field content interpreted using ['sql|C'] option.
However this is only the read issue. We have to create different say ['sqlw'] option for writting. This actually affect only add/change actions, but it has to be certainlly different option to ['sql'], because it can make a lot of confusion (for example setting ['sql|C'] should apply for change page display or change page save?)
I hope you understand me. Let me know what do you think about this. |
|
|
hbernard
2003-08-16 23:06
|
As I understand you,
'sql' would be the expression for displaying (OUTPUT)
'sqlw' would be for inserting/updating, regarding the input value (INPUT)
I this what you mean ?
(if I set "A" in my field, sqlw can cause to insert "a", but 'sql' can cause to display 'A' every where)
You certainly think about complex input mask handling, uh ?
I already made a patch around 'sql' option, but missed your point because my goal was quite different.
We certainly can mix both approach.
Here is what I have for now :
'sql' option as you defined it.
'virtual' (true/false) :
- require a 'sql' option
- force the field to be readonly
- never processed
- this field doesnt exist in table's fields. It's a pure expression, depending on other field value (using 'sql' definition).
Its an output/query only field
- if this virtual field is key field, do some specific stuff
'autoset' (true/false) :
- require a 'sql' option
- force the field to be readonly
- cant be autoset if already 'virtual'
- allways processed
- if this autoset field is key field, do some specific stuff
- this field exists in table's fields, when inserting/updating, the 'sql' expression value is inserted/updated.
+ :
- add some stuff to handle 'sql' case where PMEtable0 is defined in sql query and where it is not. (automatic prefixing to avoid ambiguous field)
- some minor improvement to set_value_from_table (custom left join, handling of 'virtual' field in simple case
- some stuff to exploit 'virtual' field to handle conditional row css
- some stuff to autodeclare virtual field when user wants multiple key.
Total : around 50 lines added to PME.
This works very well.
- virtual multiple key is OK
- conditionnal CSS row is OK (using a virtual field)
- autoset is OK
for example, when you set up key to be Array('ID1, 'ID2'), PME set a new field with 'sql' => 'concat(ID1, "_", ID2)', virtual = true.
And that's all ;-)
Virtual field is what I want to see in PME
___
Reading your way, I can imagine this :
- if sqlw defined but not sql, use sqlw as sql
UPDATE : no, stupid me, it isn't useful
- but not the reverse
- get rid of my 'autoset' declaration, which can be emulated with 'sqlw'
- keep 'virtual' flag, which will use 'sql'
[
I suppose it's better to have separate bug report
- for conditionnal css row.
- custom left join
)
What do you think ?
I will post my patch Monday morning, because I'm currently on a RTC connexion which is sloooooooow and cut off every minute.
edited on: 2003/08/16 23:08
edited on: 2003/08/18 11:40 |
|
|
hbernard
2003-08-18 18:31
|
Uploaded :
phpMyEdit.class.php patched
know.php => sample
know.sql => 2 mysql tables for testing
sample includes :
- dual keys (virtual)
- conditionnal css
- sql/sqlw stuff (year is displayed but 2003 - year is stored)
Please give me some feedback, play with it, etc...
Will post a commented diff tomorrow, too much work for now (Blaster....) |
|
|
nepto
2003-08-18 22:36
|
I just commited a revision, which makes ['sql'] works in all modes. Of ocurse, this is limited for read from database.
I will review your changes this night or tommorow.
Thank you for your interest and activity. |
|
|
hbernard
2003-08-19 13:58
|
OK, I'll wait for your comments.
When we agree on implementation, I 'm volontar to make definitive patch against current CVS. |
|
|
hbernard
2003-08-25 16:27
|
Did you have time to review this ?
Please, I have a lot of patches waiting, but I need to submit step by step in order not to have lot of merging work. |
|
|
hbernard
2003-10-07 11:45
|
just uploaded fresh diff against current CVS
+ commented diff
know.php / sql sample previously uploaded is still valid against patched phpMyEdit
Please, give me some feedback |
|
|
nepto
2003-10-07 19:15
|
Sorry, I'm lost in your changes.
But however I'm willing to integrate them (after some alternation of course). Now I'm enough estabilished/stabilised in my employment and I can deserve some time again to open source.
One very important question at the end:
The posted patches are implementation of your specification above posted in
2003/08/16 23:06? |
|
|
hbernard
2003-10-08 10:27
|
the sample know.php + know.sql is still valid.
diff.txt implements against v1.94 features discussed below :
* sqlw field option
sqlw value is a sql expression, which is used when you insert or update records.
there are some hack to make this work with key fields, (the "changemore" isn't simple to handle when your key field is a sqlw one.)
the field 'sinceyears' in know.php is a sample of use (if INPUT is x, we store 2003-x in database)
$val in 'sqlw' string will be replaced by the INPUT value when inserting or updating
* virtual field option
must be use with 'sql' option.
allow to have some "field" that don't exists in the table, but are displayed as is.
this force the field to be readonly.
+ some hack to make this work with virtual key field ( a typical virtual key field is a field with 'sql' set to concat other fields)
$opts['fdd']['ID'] = array(
'name' => 'ID',
'sql' => 'concat(ID1, "_", ID2)',
'virtual' => true)
+ multiple keys handling which is an internal implementation of code above.
See the know.php to have a simple sample of multiple key,
See the diff.txt to see how to
* conditionnal css row/cell handling :
with global option (see know.php) you can tell phpMyEdit to use a field value as a css class name.
typical use is to have a hidden virtual field with 'sql' expression which will compute css class name.
see know.php.
* aliasing :
PMEtable0 is now defined in one place. the var is then use everywhere.
you may now use ${tb_alias} in sql/sqlw string to prefix fields name.
this is convenient but also mandatory if you deal with complex sql expression
(depending of querys, PME use or don't use PMEtable0. Using $tb_alias, PME will subst either 'PMEtable0' or '', depending of the context)
I hope you will understand me |
|
|
nepto
2003-10-08 21:54
|
OK, autoset is gone? That's what I wanted to suggest.
I'm not satisfied with having field content as CSS class name. There has to be different way for handling this, because this is not too conceptional.
However I will probably apply your patch without this feature.
Btw. why there is $val in ['sql'] and ['sqlw'], when user can manualy write field name into these options? It's because of unclear "PMEtable0" aliasing usage or there is also some good argument for dooing this? I'm asking this, because every eval() is dangerous and there must be a really good reason for adding new one.
Thank you. |
|
|
hbernard
2003-10-09 08:56
|
Yes, autoset is gone since 2003-08-18 in my devel code ;-)
1. About Conditionnal CSS handling
I'm unsure if my explanations were sufficient.
If you have a bug tracker like platon.sk, you're wanting to compute css-row style depending of row content.
You could compute this using custom-php functions, but if your style depends of more than one field, the generic implementation must be able to handle all the fields, uh ?
so, the idea was to use the 'virtual' feature to compute a sql expression.
But maybe you want to handle this internally ?
API could simply be
$opts['css']['row-sql-name'] = <<My SQL expression>>
If MySQL expression is NULL, then use the default.
Internally, it could still be a virtual field with sql expression, but PME users won't see it...
2. About unclear PMEtable0 aliasing
PLease note that this is not directly $val related.
Depending of query, PME use or not PMEtable0. prefixing.
When you got ambiguous field name, this is a problem.
PMEtable0 aliasing is a work around to give user a unique way to name fields in its querys.
3. About $val
you write :
<< Btw. why there is $val in ['sql'] and ['sqlw'], when user can manualy write field name into these options? >>
please not that $val is replaced by the input _value_ not the input _field_name_
Without it, you can't use the input value to compute the new field value.
eval() was the
- easiest way
- coherent with other part of PME.
but, I agree with security concerns.
It is also possible to make string remplacements.
[PME_INPUT_VALUE] can then be a good candidate.
then, you have to make PMEtable0 in the same way
so in my previous sample :
'sql' => '2003 - ${tb_alias}sinceyears',
'sqlw' => '2003 - $val',
will be :
'sql' => '2003 - [PMEtable0.]sinceyears',
'sqlw' => '2003 - [PME_INPUT_VALUE]', |
|
|
hbernard
2003-10-12 12:47
|
Reading #127, I think the 'virtual' field property can certainly apply either to 'sql/sqlw' options or 'php/phpw' options. |
|
|
hbernard
2003-10-27 17:32
|
Nepto, do you plan to apply this patch very soon ?
Or do you want another patch with string replacement in place of eval' ?
Or you may give me cvs access in order to allow direct committing.
If so, I will allways check that you agree with patches, of course. |
|
|
nepto
2003-11-12 10:58
|
Hm, I tried your files with know.php example. It worked well, however I still do not understand how things are worked.
I have especially fear from these lines:
$newValue = "@PME_SQLW_" . $fd . ":=" . $this->print_sql_sqlw_string($this->fdd[$k]['sqlw'],false,$fn);
$query_real .= ',' . $fd . '=IF("foo"=@PME_ID:=' . $this->print_sql_sqlw_string($this->fdd[$this->key_num]['sql'],false) . ',' . $newValue . ',' . $newValue . ')' . '';
For example during update operation, the generated query looks like this one:
UPDATE know
SET
know_id='5',
known_id='5',
sinceyears=@PME_SQLW_sinceyears:=2003 - 1965,
sinceyears=IF("foo"=@PME_ID:=concat(know_id,"_",known_id),
@PME_SQLW_sinceyears:=2003 - 1965,
@PME_SQLW_sinceyears:=2003 - 1965)
WHERE (concat(know_id,"_",known_id)="3_3")
While I would expect something like this:
UPDATE know
SET
know_id = '5',
known_id = '5',
sinceyears = 2003 - 1965,
WHERE know_id = 3 AND known_id = 3
I had to and I will investigate further.
edited on: 2003-12-30 22:07 |
|
|
hbernard
2003-11-12 11:08
|
|
|
|
hbernard
2003-11-12 11:13
|
to be more explicit, in change operation :
... (Line 2331)
if ($query_sqlw == '') {
$query_sqlw .= 'SELECT @PME_SQLW_'.$key;
} else {
$query_sqlw .= ', @PME_SQLW_'.$key;
}
...
Create the query to retrieve @PME_SQLW_* value
This query is run in :
....(Line 2358)
if (array_count_values($changed_sqlw)) {
$res = $this->myquery($query_sqlw, __LINE__);
$sqlwvalues = @mysql_fetch_array($res, MYSQL_ASSOC);
foreach ($changed_sqlw as $key) {
$newvals[$key] = $sqlwvalues['@PME_SQLW_'.$key];
}
}
...
and newvals[] array is completed. |
|
|
nepto
2003-12-08 11:03
|
Experimetal ['sqlw'] stuff was commited into CVS. Please tests and report if it correctly works.
Besides some trivial "virtual" fields stuff no other things from your patch were introduced into CVS. They probably deserves their own bug report. |
|
|
hbernard
2003-12-08 11:08
|
ok, I will test it this week |
|
|
hbernard
2003-12-08 21:33
|
__(Ln 385)
There is a duplicate line :
$qparts['from'] = $this->db.'.'.$this->tb;
___
> $val = $newvals[$this->fds[$k]];
> $val_as = addslashes($val);
> $val_qas = '"'.addslashes($val).'"';
interesting, what was your use case for adding this ?
____
I will fill in some _individual_ bugs/feature for :
- feature: conditionnal css handling (you removed)
- bug: PME behavior with manual key or sqlw key ( (you removed)
- feature: using virtual key field ((you removed)
- feature: explicit "LEFT JOIN" statement. (I agree, it was a quick hack)
- bug: dealing with ambiguous field name with 'values' options (which was the reason to allow aliasing)
Please consider the #114, which I really think is ready to commit (but maybe not, it seems you dislike my patches ;-) |
|
|
nepto
2003-12-10 22:31
|
Ad 1: Duplicate line removed.
Ad 2: In the CVS version of the documentation is everything written well. ;-) You have three variables available: $val, $val_as ("as" for addslashes()) and $val_qas ("qas" for quoted addslashes()). You can choose in your ['sqlw'] option whatever you want.
However I'm considering to remove *all* evaluations in phpMyEdit before next release. This does not mean only ['sqlw'] evaluates, but also ['URL'] evaluates and so on. Yes, I know this would breaks BC.
Ad 3: It's definitelly good idea to submit several smaller bug-reports for the different issue, even if they are small. You patch was complatelly probably good, but I have to implement only ['sqlw'] (for now), thus it was really difficult task to go through whole patch and select only relevant parts.
It's not about liking or disliking patches. Core class is huge (near 100 kB now) and the code looks sometime like mess. We have to keep things there clean and tidy and well-documented, otherwise phpMyEdit can soon became unmaintainable.
As I already say: several reports, everyone dealing with its own thing, where such issue can be deply discussed. I consider this bug #121 as pure ['sqlw'] only, but that does not mean, that your other ideas are wrong or bad. They simply does not have nothing to do with ['sqlw'].
I will left this item open until ['sqlw'] would not be at least partially tested and proved to work. Concerning to other features:
[1] virtual fields - definitelly very good idea, which will solve a lot of problems, some basis are already commited and the rest will be implemented soon
[2] conditional css handling - yes, but somewhat via $opts['css'] -- I have some idea
[3] multiple fields primary key handling - I do not want to emulate this via ['sqlw'] for several reasons; otherwise I would like to see array-like initialization of these primary keys
[4] explicit LEFT JOIN - I did not understant this :-/ |
|
|
hbernard
2003-12-31 09:49
|
Answering to forum topic : http://Platon.SK/forum/projects/viewtopic.php?p=817#817
I wonder if this should not be in the documentation :
(English is very bad, I m not sure about "projective expression")
____________________________
Previous samples are projective expressions, which means :
TRIM(TRIM(" Some text here ")) == TRIM(" Some text here ")
UPPER(UPPER("my sample Text")) == UPPER("my sample Text")
What about sqlw expression that allways change the value ? (ie, non projective expression)
If you have a sqlw expression (noted sqlw()) for which the property below is true,
sqlw(sqlw(A_VALUE)) != sqlw(A_VALUE),
___
Case 1 : there is a reverse func that can be applied before displaying the form :
(bijective expression)
For example :
Your users are entering a value in field 'myfield' that is processed with
'sqlw' => '$val + 10',
Then you have to apply the reverse before displaying the change form
'sql' => 'myfield - 10',
User ADD input : 32
Mysql stored value (using slqw): 42 (32+10)
Value displayed in change form (using sql) : 32 (42-10)
Mysql stored value (using sqlw again):42 (42+10)
___
Case 2 : There is no reverse function (typically, hashing functions)
then you will have to test if value was changed, in order not to apply sqlw() when this is not required.
For example, using the md5() function :
You have to use this code
$opts['fdd']['myfield'] = Array(
...
'sqlw' => 'if(myfield=$val_qas,$val_qas,md5($val_qas))',
...
);
in order to avoid re-hashing an already hashed string.
This mean, if 'myfield' value was not changed, then don't apply md5() on it.
If 'myfield' value was changed, then apply md5() on it. |
|
|
nepto
2004-01-20 12:06
|
Yes, right. The second example was very good, I added it into documentation.
I hope we could close this issue and follow opened issues in the another reports. |
|
|
hbernard
2004-01-20 21:05
|
I can't see any reason to not consider this bug as resolved |
|
|
hbernard
2004-01-21 14:03
|
well, I have to reopen this bug because of following API misbehavior.
Before sqlw, all options for a field were applyed during the same pass.
I mean, all 'trim', 'values', 'tab' (etc... where used during the same overall operation.
Option-variability (the pme_option|ACF... syntax) was consistent.
Now we have sqlw, I went into this problem :
I have a specific sqlw operation that I want to be performed only when updating (not inserting) record.
Then, I wrote 'sqlw|C' => [my sql stuff],
This didn't work.
Why ?
Because 'UPDATE' SQL operation is not performed during PME 'Change' operation, but during savechange or morechange "operation".
Of course, If I put 'sqlw|LFC' => [my sql stuff], then it works as expected, but it seems a bit odd to have to allow sqlw in LF mode .....
I think the fix could be in the recreate_fdd() func,
-$this->change_operation() && $this->page_type = 'C';
+($this->change_operation() || $this->savechange == $this->labels['Save'] || $this->morechange == $this->labels['More']) && $this->page_type = 'C';
and same things for add_ and copy_
BUT then, we have to recreate_fdd() after UPDATE SQL statement, and I wonder what else.
Another fix could be to make change_operation() behave correctly,
function change_operation() { return ($this->operation == $this->labels['Change'] || $this->savechange == $this->labels['Save'] || $this->morechange == $this->labels['More']) && $this->change_enabled(); }
But I'm a bit worried about side-effects in list_operation(), etc...
So I'm lost. Has somebody some thought about this.
I hope you understand the problem. If not, please tell me, this is a real concern. |
|
|
hbernard
2004-01-21 22:18
|
uploaded : "hb-sqlw-recreate.diff"
This fix the bug described in the bugnote above.
I choosed the first way (changing recreate_fdd() func).
What do you think about this ? |
|
|
hbernard
2004-01-22 10:23
|
Patch uploaded before is not absolutely correct.
Uploaded new hb-sqlw-recreate2.diff, I hope this is the last one.
Here are more rationnales for this :
__________________
Current behavior :
PhpMyEdit has 3 passes :
PAss3 (the first processed) is updating/inserting/deleting => ??????
PAss2 is Add/Copy/chage/delete/View FORMS => 'ACPVD'
Pass 1 is list => 'LF'
When necessary Pass3 is allways processed just before
- Pass2 (morechange, moreadd)
- or Pass1 (savechange, saveadd, savedelete)
(in the same "page_mode" name space).
Options Variability make 'L' the default, so during pass3 we are in List/filter mode
(because we are not doing an 'operation' listed in the Pass2 - Add/Change/etc...)
This will break 'sqlw' behavior when using options variability. (ie : when sqlw is not intended to have same value during insert and update sql-operation)
How is it broken ?
'sqlw|C' has no effect because 'sqlw' is not performed during outputting Change Form(pass2), but during pass3.
'sqlw|A' has no effect for same reasons.
'sqlw|LF' (non sense API) will perform 'sqlw' during Inserting AND Updating
This mean also :
'sqlw|AC' has no effect
'sqlw|LA' === 'sqlw|L' === 'sqlw|LC'
'sqlw' is currently the only thing affected by this
In the near future, phpw can also be candidate for this bug.
____________
How to fix ?
From an API point of view,
'sqlw|C' should be correct to have sqlw performed only during update.
same for 'sqlw|A'
same for 'sqlw|P'
'sqlw|LFDV' should have no effect.
the recreate_fdd is called at the very beginning of the PME script
fix is to make recreate_fdd set page_mode to 'A' or 'C' or 'P' when insert/update has been requested.
(see the patch)
But then, after inserting/updating, we will leave pass3 and enter either pass2 or pass1.
entering in pass2 is ok (fdd recreate is already called in case of 'more...' stuff)
but before entering in pass1 (list mode), recreate should be called with page_mode set to 'L'
patch fix the missing recreate_fdd call, $this->save[add/change]are unset before recreating.
finally, because saveadd is used in both 'A' form and 'P' form, you can't make any distinction between.
solution is to have a savecopy button (in place of saveadd).
I hope this is correct. |
|
|
hbernard
2004-02-02 14:04
|
Nepto, I think this is maybe the last issue before releasing 5.4.
Can you have a look at this ? |
|
|
hbernard
2004-04-14 14:06
|
nepto, there is no reference to 'sqlw' in the patch because this is a more general problem. (See explanations above)
I remember another sample :
in a simple table, on a field, try these options :
'options|C' => 'C',
'options|LF' => ''
This should allow to edit and change the field, without viewing it in list mode.
This doesn't. I mean, you can't change the field. update is not performed.
That's because when we are in pass3 (updating), page_mode is to default 'L', and then the field is not processed. (see bugnotes above)
The (quite old, but valid) patch adress this by making sure to have real mode set when we are in pass3.
Please comment.
edited on: 2004-04-14 14:09 |
|
|
nepto
2004-04-14 20:44
|
Yes, you correctly grasp the problem.
I know about this few months and this causes me a little headache.
When I was thinking about the solution, I come to two possible solutions:
1. Introduce new flags for insert/update/delete actions (not pages).
Thus you will specify something like ['option|ACPIU']
ACP for Add/Change/Copy and IU for Insert/Update
2. Make some intelligent recreate_fdd() processing. This means, that when action (insert/update/delete) is performed current page type will be according to page from which was values submitted (add/copy/change/delete). Thus ['option|C'] will be considered during update operation.
After insert/update/delete is done, page type is set to what is actually displayed (mostly "list/filter", but can be also "add" when "More" was pressed or "change" when "Apply" was pressed) |
|
|
hbernard
2004-04-14 21:01
|
(2) is what the patch was doing.
It could be much more readable if "page_mode" was a persistent cgi var. |
|
|
nepto
2004-04-14 21:48
|
"savedelete" forgotten; I will correct before commit |
|
|
nepto
2004-04-14 22:15
|
OK, commited. To have it complex, "cancelchange" was also introduced.
Please tests, if your ['sqlw'] works as expected.
We can also check
I remember another sample :
in a simple table, on a field, try these options :
'options|C' => 'C',
'options|LF' => ''
And we should carefully watch any side effect this source update may brings.
Thinking about this a while, I believe no unwanted effect will occur. Only options used in do_(add|change|delete)_records() are affected (such as ['sqlw']), what is in fact desirable. |
|
|
hbernard
2004-04-14 22:38
|
check ok for both (options and sqlw). Many thanks, I may now upgrade to not patched cvs version. |
|
|
hbernard
2004-04-17 12:24
|
uploaded cancelcopy.diff.
There was a typo (cancelchange in place of cancelcopy).
Please Commit. |
|
|
nepto
2004-04-17 13:23
|
Wow, we have this finally resolved!
Thank you for catching my typo, I repatched it in the CVS.
Well, updates should not be done in this "direct" way, however this is the only exception. |
|
|
|
|
Prihlásiť
Registrácia
English
