orango icon indicating copy to clipboard operation
orango copied to clipboard

Schema Indexing Documentation + PersistentIndex Bug

Open armenr opened this issue 6 years ago • 6 comments

Hey there - it's your friendly neighborhood annoying dude who's loving your ODM and perpetually blowing up your Issues section.

Working on a model, and I want to add an index of type HASH to a given field, and then ensure uniqueness on that index. This would apply to SkipList indices, as well, where this same kind of uniqueness enforcement might be useful.

I got curious and figured I'd investigate for the other known types, and provide the most complete possible bug report back to you.

Summary:

HASH Index type

  • passing sparse: true/false on HASH index silently doesn't work (always defaults to false)

  • passing unique: true/false on HASH index silently doesn't work (always defaults to false)

FULLTEXT type

  • passing sparse: true/false on fulltext index doesn't appear to work

PERSISTENT

Persistent type index doesn't appear to work at all, and doesn't trace back to which model is throwing the error

[myproj-api] (node:50597) UnhandledPromiseRejectionWarning: TypeError: collection.createPersitentIndex is not a function
[myproj-api]     at Orango.ensureIndexes (/Users/armenr/Desktop/myproj-new/myprojzer0/node_modules/orango/lib/Orango.js:461:36)
[myproj-api]     at processTicksAndRejections (internal/process/task_queues.js:89:5)
[myproj-api]     at Orango.createCollection (/Users/armenr/Desktop/myproj-new/myprojzer0/node_modules/orango/lib/Orango.js:410:7)
[myproj-api]     at Orango._createCollection (/Users/armenr/Desktop/myproj-new/myprojzer0/node_modules/orango/lib/Orango.js:226:9)
[myproj-api]     at async Promise.all (index 9)
[myproj-api]     at Orango._processModels (/Users/armenr/Desktop/myproj-new/myprojzer0/node_modules/orango/lib/Orango.js:98:7)
[myproj-api] (node:50597) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 1)
[myproj-api] (node:50597) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

SKIP_LIST

  • passing sparse: true/false on SKIP_LIST index silently doesn't work (always defaults to false)

  • passing unique: true/false on SKIP_LIST index silently doesn't work (always defaults to false)

General Behaviors/Other

  • changing a Schema's index from one type to another e.g. -fulltext --> hash, leaves the previous hash index intact and simply adds the new fulltext index without cleaning up the old fulltext index (I figure this one might be intended for safety reasons?)

  • changing an existing schema's existing FULLTEXT index from sparse: true to sparse: false doesn't change/recreate the index, and simply does nothing, silently (no warning or error). It only works if you recreate the collection. (I figure this one also might be intended for safety reasons?)

Code Stuff:

In any case, here's my code:

const UserModel = (orango) => {
  // const { OPERATIONS, SCHEMA } = orango.consts;
  const { SCHEMA } = orango.consts;

  const schema = new orango.Schema(
    {
      firstName: String,
      lastName: String,
      username: String,
      password: String,
      isVerified: Boolean,
      verificationCode: String,
      active: Boolean,
      email: String,
      // tags: [String],
      // role: { type: String, default: 'user', defaultOnUpdate: 'admin' },
      created: { type: Date, default: Date.now },
      updated: { type: Date, defaultOnUpdate: Date.now },
      settings: 'Settings',
    },
    {
      indexes: [
        {
          type: SCHEMA.INDEX.HASH,
          fields: ['email'],
          unique: true,
        },
      ],
    },
  );

I have verified that iterations of testing and rerpoting with this sort of sample command from arangosh, as well as inspecting the web-UI

127.0.0.1:8529@dev1> db.users.getIndexes()

Sample output:

[
  {
    "deduplicate" : true,
    "fields" : [
      "email"
    ],
    "id" : "users/677317",
    "selectivityEstimate" : 1,
    "sparse" : false,
    "type" : "hash",
    "unique" : false
  }
]

Thanks again! You might be tired of hearing this crap from me, but on behalf of my team: We're grateful for all the hard work you've done to build this thing by yourself. I've been inspecting the code in your repo, but I'm so strapped with work, I'm having a rough go of trying to patch this up myself, or contribute with my own PR.

armenr avatar May 18 '19 12:05 armenr

I can confirm that the standard ArangoJS driver works for this. I went snooping around the OrangoJS codebase, and I think the problem is as simple as the case being that in the createIndex() call that gets passed into the ArangoJS driver, Orango isn't passing the options object.

I thiiiiiink, this little function right here might be the culprit: https://github.com/roboncode/orango/blob/654c0dc3b6b1d318aaa83e32675a3db2207e29a6/lib/Schema.js#L89

It doesn't appear to be parameterized to accept, inspect, or return any of the optional arguments for uniqueness, sparsity, etc.

Additionally, when I log the "item" being received in file Orango.js, "item" comes in without any item.opts:

https://github.com/roboncode/orango/blob/654c0dc3b6b1d318aaa83e32675a3db2207e29a6/lib/Orango.js#L391

const arangojs = require("arangojs");
const db = new arangojs.Database();

console.log("DATABASE IS ", db)
db.query({
        query: "RETURN @value",
        bindVars: { value: now }
    })
    .then((cursor) => {
        return cursor.next().then((result) => console.log(result))
    })
    .catch((err) => console.log(err))

const collection = db.collection('potatoes');
collection.create()

collection.createIndex({
    type: "hash",
    sparse: true,
    unique: true,
    fields: ['email'],
});

Checked and confirmed working:

127.0.0.1:8529@_system> db.potatoes.getIndexes()
[ 
  { 
    "fields" : [ 
      "_key" 
    ], 
    "id" : "potatoes/0", 
    "selectivityEstimate" : 1, 
    "sparse" : false, 
    "type" : "primary", 
    "unique" : true 
  }, 
  { 
    "deduplicate" : true, 
    "fields" : [ 
      "email" 
    ], 
    "id" : "potatoes/686969", 
    "selectivityEstimate" : 1, 
    "sparse" : true, 
    "type" : "hash", 
    "unique" : true 
  } 
]

I'm working on patching this. If I come up with something that's decent, I'll open a PR and stop being a whiny baby :)

armenr avatar May 19 '19 13:05 armenr

Figured out a bunch of stuff.

Index options actually do work!

We just have to append "opts" to the index, like in the code snippet, below. So, that's my bad.

Persistent Index Bug

Persistent Indices fail only because of a very simple typo on line 461:

promises.push(collection.createPersitentIndex(item.fields, item.opts))

I will fork with a proposal to update documentation, and also to correct the spelling of createPersitentIndex to createPersistentIndex.

The following will work for all index types that optionalize uniqueness and/or sparsity:

  const schema = new orango.Schema(
    {
      firstName: String,
      lastName: String,
      username: String,
      password: String,
      isVerified: Boolean,
      verificationCode: String,
      active: Boolean,
      email: String,
      // tags: [String],
      // role: { type: String, default: 'user', defaultOnUpdate: 'admin' },
      created: { type: Date, default: Date.now },
      updated: { type: Date, defaultOnUpdate: Date.now },
      settings: 'Settings',
    },
    {
      indexes: [
        {
          type: SCHEMA.INDEX.HASH,
          fields: ['email'],
          opts: { sparse: false, unique: true },
        },
      ],
    },
  );

armenr avatar May 19 '19 17:05 armenr

So does it mean it has unique together as it's supports array.

indexes: [
        {
          fields: ['email', 'username'],
          opts: { sparse: false, unique: true },
        },
      ]

iraycd avatar May 20 '19 05:05 iraycd

^^ The above code works, but you'll have to specify which Index TYPE you want to use (Hash, etc).

I can verify as I am using the same exact setup. BUT, be certain that this is what you want.

In this case, you're saying: I want you to generate a hash from the username + email values, and ensure that the combination of them is always unique. That's very different from saying - do not allow duplicate emails, and also do not allow duplicate usernames.

That would require you creating two, separate indices:

      indexes: [
        {
          type: SCHEMA.INDEX.HASH,
          fields: ['email'],
          opts: { sparse: false, unique: true },
        },
        {
          type: SCHEMA.INDEX.HASH,
          fields: ['username'],
          opts: { sparse: false, unique: true },
        },
      ],

armenr avatar May 20 '19 06:05 armenr

I mean don't allow only if combination of the both is same.

iraycd avatar May 20 '19 07:05 iraycd

In that case, yes. Your first example was correct, with the following correction:

indexes: [
        {
          type: SCHEMA.INDEX.HASH,
          fields: ['email', 'username'],
          opts: { sparse: false, unique: true },
        },
      ]

armenr avatar May 20 '19 20:05 armenr