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

Undetected data dependencies #6

Closed
2 tasks done
spenceryue opened this issue Aug 2, 2023 · 3 comments
Closed
2 tasks done

Undetected data dependencies #6

spenceryue opened this issue Aug 2, 2023 · 3 comments

Comments

@spenceryue
Copy link

spenceryue commented Aug 2, 2023

Description

There are many ways to express data dependencies that aren't discoverable by the DAG building process.

Actual Behavior

One example

> .init -f "test.json"
{
  "a": "",
  "b": [
    "/${a}"
  ],
  "c$": "$lookup('a')"
}
> .out
{
  "a": "",
  "b": [
    ""
  ],
  "c$": ""
}
> .set /a "✅"
{
  "a": "",
  "b": [
    ""
  ],
  "c$": "" // 👈 Actual
}

Expected Behavior

.set /a "✅" should result in "c$": "✅".

> .set /a "✅"
{
  "a": "",
  "b": [
    ""
  ],
  "c$": "" // 👈 Expected
}

Affected Version

0.0.10, 0.0.29

Steps to Reproduce

  1. Clone the repo at latest (1452aa6).
  2. Change // to # in .npmrc. (Otherwise next step fails.)
  3. Run yarn.
  4. Create test.json with above snippet.
  5. Run yarn stated.
  6. Enter above commands (.init -f "test.json", .out, .set /a "✅").

Checklist

Environment

node -v yarn -v
v18.13.0 1.22.19
@spenceryue
Copy link
Author

spenceryue commented Aug 2, 2023

Other examples for c$ that result in the same .out and .set behavior:

  • $eval('a')
  • b.%.a
  • ($ ~> | $ | { 'a': a } |).a
  • function ($it) { $it.a }($)
  • ($it := $; $it.a)
  • b[0 + 0]
  • b.( function() { $[0] }() )
  • $sift(function ($_, $key) { $key = 'a' }).a
  • *[0]

@geoffhendrey
Copy link
Contributor

Hi spencer - Stated does not try (at present) to do semantic analysis of functions. Rather than trying to hook in, I feel the best thing to do is just this:

{
  "a": "❌",
  "b": [
    "/${a}"
  ],
  "c$": "(a;$lookup('a'))"
}

In other words, just manually add the dependency to c$. There will alway be a class of dynamic expressions. We find these to be rare. So rare in practice that they have not occurred in any real uses cases so far. But we have anticipated and the solution is pretty straightforward manual dependency setting like I showed above. Thanks!

@spenceryue
Copy link
Author

(Copied here from webex for completeness)

When a developer doesn't control all the code (e.g. imports), it may be difficult to reason about when a dependency needs manual declaration.

It's also not just functions that break analysis. Computed array indices (b[0 + 0]) and parent traversal (b.%.a) also currently evade analysis. (But, I would say functions are even more basic than these, since they are needed to deduplicate code.)

To clarify, my concern is about correctness, not feature richness. I think it's fine to say "we don't support X and Y". But presently, the issue is it's impossible to tell when an undetected dependency is a bug or "working as expected". I expect there to be a flood of questions as usage scales up. And if stated is to be a foundation of other user facing APIs, the problem is those APIs will receive floods of questions as well.

I'm concerned as a maintainer of those user facing APIs, that I will be pressured to adopt stated at the cost of increasing my own maintenance burden, if these correctness issues are not addressed.

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

No branches or pull requests

2 participants