Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WriteQueries :: UpdateByWhere #57

Open
Pierozi opened this issue Dec 3, 2015 · 12 comments
Open

WriteQueries :: UpdateByWhere #57

Pierozi opened this issue Dec 3, 2015 · 12 comments

Comments

@Pierozi
Copy link

Pierozi commented Dec 3, 2015

Hello, i don't know if this idea has been already discussed,
but i think that could be helpful to have an method updateByWhere.

I've done this method for me

    public function updateByWhere(Where $where, Array $params) {

        $entities = $this->findWhere($where);

        foreach ($entities as $entity) {

            foreach ($params as $key => $value) {
                $entity->$key = $value;
            }

            $this->updateOne($entity, array_keys($params));
        }

        return $entities;
    }
@chanmix51
Copy link
Member

Hello @Pierozi

There have been a lot of discussions about this method and the solution you present is one of the reasons there are no such method 😄

In your method, all the relevant records are fetched from the database and updated one by one, this is a typical object oriented instance in mind approach resulting in a nested loop. This is a design flaw issuing as many requests as rows to be updated.The relational solution of the problem would be something like:

UPDATE {relation} set {field} = $* WHERE {where} RETURNING {projection}

But we could not find out a way to code a generic update such as field = my_func(another_field) or SET field1 = $*, field2 = $*.

The complexity of the different UPDATE cases is a reason why there is no such method … yet.

@chanmix51
Copy link
Member

Sorry, I accidentally closed the issue. I let it open for discussion as it would be nice if we could figure out a solution for this.

@Pierozi
Copy link
Author

Pierozi commented Dec 3, 2015

Yes you right, my case have a Where with limited row returned and it's execute in worker.

But for large update, the best case should create temporary table, fill in, and then make update by sql request, this way can be done easily with the Where no ?

@chanmix51
Copy link
Member

I do not understand why a temp table would be needed. The most complex cases can be handled by a CTE.

@Pierozi
Copy link
Author

Pierozi commented Dec 3, 2015

Right, still not the reflex to think about CTE.

first though was we need it for the complex condtion like SET foo = my_func(TMP.foo)

@chanmix51
Copy link
Member

You can join sets in an UPDATE statement, see the from-list paragraph in the documentation.

@tlode
Copy link
Contributor

tlode commented Dec 3, 2015

Last time I thought about updating entities, I came up with a idea to rewrite the Where helper and let it base on a similar concept named Expression. The idea was to use updateWhere like this

  /**
   * @param Expression[] $expressions
   * @param Where[] $conditions
   */
   public function updateWhere(array $expressions, array $conditions)

An Expression could be simple a a = 1 term, but also a a = $*, a = b + 1 or a = myfunc(c). Expression should be chainable like Where is, but in a more generic way. Where would just be a specialized case of an Expression, offering AND, OR, etc. chaining methods (probably the stack engine from Where could therefor be used within Expression for chaining).

I already did some work on it some time ago, but I stopped, because I haven't had anytime anymore. If you think it is worth it, I could review how far I came...

@chanmix51
Copy link
Member

👍

Something like

    $expr = (new UpdateExpression('a = 1'))
        ->add('b = a + $*::int4', [4])
        ->add('c = sql_func(b -a)')
        ;

    sprintf("… SET %s WHERE …", $expr);
    // return "… SET a = 1, b = a + $*::int4, c = sql_func(b-a) WHERE …"     
    $expr->getValues(); // return [4];

Could even be:

    $expr = (new UpdateExpression(['a' => 1, 'b' => 'a + $*::int4'], [4])
        ->add(['c' => 'sql_func(b-a)'])
        ;
    $expr->getUpdatedFields(); // ['a', 'b', 'c']
    $expr->getUpdatedExpressions(); // [1, 'a + $*::int4', 'sql_func(b-a)']
    (string) $expr; // a = 1, b = a + $*::int4, c = sql_func(b-a)
    $expr->getValues(); // [4]

Any thoughts ?

@tlode
Copy link
Contributor

tlode commented Dec 3, 2015

👍

Still, I believe that even UpdateExpression is still a specialized implementation of a more generic Expression object. It could generally introduce handling of numbered parameters like a = $1 or even named ones like a = $foo. But this is another task.

I think it is best to go with specialized first :-)

@chanmix51
Copy link
Member

I do fully agree with you, we have to design APIs with UX in mind 👷 scrutinizer will yell at us if we do not share common code between Where and UpdateExpression anyway…

@chanmix51 chanmix51 added this to the 2.1 milestone Dec 3, 2015
@ghost
Copy link

ghost commented Dec 3, 2015

I fully agree with this kind of implementation for UpdateExpression() 👍

@chanmix51
Copy link
Member

Function must be kept simple for simple operations:

    $model->updateWhere(
        ['field_b' => null, 'field_d' => 'done'],
        Where::create('field_a < $*::timestamp', [new \DateTime])
        )
    /*
    UPDATE {relation}
    SET
      field_b = $*::bool,
      field_d = $*::varchar
    WHERE field_a < $*::timestamp
    RETURNING {projection}
    */

More complex cases could be explicitly described. Variable escaping and type hinting should then be present:

    $model->updatewhere(
        new UpdateExpression(
            [
                'field_a' => '$*::timestamptz',
                'field_b' => '$*::bool and field_c',
                'field_d' => 'func_sql($*::varchar)',
            ],
            [
                new \Datetime(),
                null,
                'done',
            ]
        ),
        Where::create(
            "field_a < $*::timestamp and field_b is not null",
            [new \Datetime()]
            )
        );
    /*
    UPDATE {relation}
    SET
      field_a = $*::timestamp, 
      field_b = $*::bool and field_c,
      field_d = func_sql($*::varchar)
    WHERE
      field_a < $*::timestamp
      AND field_b is not null
    RETURNING {projection}
    */

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants