arthurhsu / rdb

Draft for W3C Relational Database Proposal
https://arthurhsu.github.io/rdb/
20 stars 0 forks source link

Feedback for current IDL spec #1

Closed freshp86 closed 7 years ago

freshp86 commented 8 years ago

Some initial comments. Have not gone through the whole API yet.

arthurhsu commented 8 years ago
freshp86 commented 8 years ago

Regarding fixed strings vs enums. I am suggesting to allow user to pass a raw string, but to also give an alias to those raw strings such that users don't have to pass the string literal. There was an example of this in the FileSystem API, where there were two constants TEMPORARY and PERSISTENT, with values 0 and 1 (they still exist if you try it in Chrome). In that case it sucked because they were directly on Window polluting everyone's namespace.

I understand there might be a precedent in W3C, but I think this is a bad precedent. The proof is the amount of wrapper APIs that exist to define enums for W3C's string literals, for example (from Closure library)

  1. goog.dom.TagName
  2. goog.db.Transaction.TransactionMode
  3. goog.events.EventType

Regarding expressing the dynamically generated schema and table classes, that is a tough one, I am not sure if there is a precedent for dynamically generated APIs in W3c, so most likely current IDL syntax does not support this.

Last thing I noticed, and() and or() and not() (not is missing currently) should not be on IPredicate. Example of the problem is that IColumn inherits from IPredicate, but emp.lastName.and() makes no sense and should not be allowed. I also think that IPredicateProvider is a better name for this interface. Predicate is something different and has its own interface (not exposed to users, but Predicate interface refers to ValuePredicate, CombinedPredicate and JoinPredicate).

arthurhsu commented 8 years ago

I've fixed the predicate issue, and I don't think join predicate should be exposed in the syntax. It's already done differently.

I vaguely remembered there were a long argument of string literals vs number. I'd say given the long spec (~300 lines IDL, probably 15K lines of first draft spec) I'll avoid minor but flammable issues like this as much as possible.

freshp86 commented 8 years ago

I am a bit confused on why do we need to expose IValuePredicate and ICombinedPredicate at all, and also don't understand ICombinedPredicate definition (why does it refers to itself?). If I am reading it correctly, one could do, myCombinedPredicate.and(foo).and(bar).and(baz)?

How about the following?

interface ILogicalOperators {
  static Predicate and(Predicate... childPredicate);
  static Predicate or(Predicate... childPredicate);
  static Predicate complement(Predicate predicate);
}

I am using static (see definition) because it makes no sense to have an instance of ILogicalOperators, it is just the equivalent of lf.op. Also with the current interfaces, a join predicate is disallowed (IColumn only inherits from IValuePredicate)?

arthurhsu commented 8 years ago

The new syntax would be

t1.id.eq(22).and(t2.id.eq(33)).and(t3.id.eq(444))

So the type is not solely correct yet. The major goal is to eliminate as many global functions as possible, which has quite a few benefits (reads better and loads on demand) that will be required as spec.

arthurhsu commented 8 years ago

Also since I'm working on spec, I'm no longer fully constraint by JS implementation details, I'll choose to further change the syntax of Lovefield if C++ can fit.

freshp86 commented 8 years ago

A static method does not mean necessarily global, it can be under a namespace (or under an object, like db.op.or, db.op.and, such that no namespace is needed). I am finding the non-static syntax more cluttered, but also harder to interact with.

// Syntax with static operator methods,  'or', 'and' appear one time each
db.op.or(a, b, db.op.and(c, d, e), f, g);  
// Syntax with non-static operator methods, 'or', 'and' must be repeated multiple times
a.or(b).or(c.and(d).and(e)).or(f).or(g);
var myPredicates = [.....];
// Static operators
db.op.and.apply(null, myPredicates);
// Non-static operators, does not look elegant
var lastPredicate = myPredicates[0];
myPredicates.slice(1).forEach(function(p) {
  lastPredicate = lastPredicate.and(p);
});
// ValuePredicate class needs to return a CombinedPredicate instance
var combinedPredicate1 = valuePredicate1.and(valuePredicate2);
// JoinPredicate class needs to return a CombinedPredicate instance
var combinedPredicate2 = joinPredicate.and(valuePredicate1);

On the other hand, when using the prefix operator syntax, CombinedPredicate only interacts with the Predicate interface. ValuePredicate and JoinPredicate do not depend on CombinedPredicate.

The non-static (infix) syntax makes more sense when parsing exists, but not so much for a declarative API, in my opinion. Anyway, we can revisit the subject once you are done working on the spec 1st iteration. I don't think I will have time to look at it until new year. BTW, here is an interesting comparison of prefix, infix and postfix operator syntax, http://www.cs.man.ac.uk/~pjj/cs212/fix.html.

arthurhsu commented 8 years ago

For the users they don't see join predicate at all (which will be a bug if they did). For the example you made: db.op.or(a, b, db.op.and(c, d, e), f, g);

The way SQL express this: a OR b OR (c AND d AND e) OR f OR g

The new syntax a.or(b).or(c.and(d).and(e)).or(f).or(g)

is actually closer to original SQL syntax.