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

Expected behaviour of push/npush/npush_if #104

Open
stdweird opened this issue Feb 11, 2016 · 2 comments
Open

Expected behaviour of push/npush/npush_if #104

stdweird opened this issue Feb 11, 2016 · 2 comments
Labels

Comments

@stdweird
Copy link
Member

While working on #102, i stumbled on some ill defined behaviour of push (which also exists in npush and push_if)

object template push;

include 'pan/functions';

"/functions/push_dml_empty/data" = {
    foreach(idx;id;list(1,2,3)) {
        push(id);
    };
};

"/functions/push_dml_nonempty/data/0" = 0;
"/functions/push_dml_nonempty/data" = {
    foreach(idx;id;list(1,2,3)) {
        push(id);
    };
};

current master returns

{
  "functions": {
    "push_empty": {
      "data": [
        3
      ]
    },
    "push_nonempty": {
      "data": [
        0,
        1,
        2,
        3
      ]
    }
  }
}

so there is a significance difference between push depending on the the existence of SELF (the reimplementation using merge as in the PR suffers from similar yet different problems. (chagning the code and ending with SELF; gives different issues altogether, but they are still different).

my question is: how to handle this. keep the old code, but warn in the doc that push/npush/push_if have undesired sideeffects in DML blocks?

some sanity could be restored by using append in push/push_if, but there is no equivalent of append for dicts (in particular, merge with single argument to update SELF is not supported; see quattor/pan#103).

@jrha
Copy link
Member

jrha commented Mar 23, 2016

We agreed at the 21st workshop that the observed behaviour is not desired or intentional.

@stdweird
Copy link
Member Author

we will fix this issue as a bug-fix in the core templates (probably by using append).
but we will not add push as a buitlin (see quattor/pan#108 for that discussion)

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

No branches or pull requests

2 participants