Giter VIP home page Giter VIP logo

Comments (12)

yajra avatar yajra commented on July 17, 2024

To begin with, are you using the latest build v1.4.2? There is already a condition to check if primary column exists before dropping the generated trigger.

        // get the actual primary column name from table
        $col = $this->connection->getPrimaryKey($prefix.$table);
        // if primary key col is set, drop auto increment objects
        if (isset($col) and !empty($col)) {
            // drop sequence for auto increment
            $this->connection->dropSequence("{$prefix}{$table}_{$col}_seq");
            // drop trigger for auto increment work around
            $this->connection->dropTrigger("{$prefix}{$table}_{$col}_trg");
        }

And yes, oracle only allows object name up to 30chars that is why you encounter ORA-00972: identifier is too long.

If your using the latest build, can you please paste your Schema script so I can test it. I am using the starter kit included in the package to test and migration works well.

from laravel-oci8.

AdamWillden avatar AdamWillden commented on July 17, 2024

I am using the latest build (I only downloaded this in the last couple of days) and it does check if its a primary key but it doesn't check if that primary key is an autoIncrement field.

Edit: Thanks for your quick reply. The phone rang as I replied so left it a bit short, sorry!

from laravel-oci8.

AdamWillden avatar AdamWillden commented on July 17, 2024

Schema Script as requested. This one is very simple as you can see:

    Schema::create('eventCurrentStatus', function  (Blueprint $table)
        {
            $table->integer('deviceGUID');
            $table->integer('statusBits');

            $table->primary('deviceGUID', 'eventCurrentStatus_PK');
        });

from laravel-oci8.

crazycodr avatar crazycodr commented on July 17, 2024

@yajra The problem is clearly and simply said that the lenght of the triggers and sequences are too long. You might want to add some shortening behavior to the generation algorithm, but there is just so much you can do because its an aggregation of many other items. You will eventually hit the limit for sure...

@AdamWillden You might want to shorten the names of your columns a bit... it is a known fact that Oracle forces an horrible 30 character limit. The two underscores and suffix "trg" and "seq" are only impacting you for 5 characters, so that means that your overal table name and columns both take up to 25 characters... Considering that all oracle developpers actually use the previously mentionned naming convention, then, a big part of all this is your responsiblity to better design tables and columns. (Trust me i hate Oracle for that though :) )

from laravel-oci8.

AdamWillden avatar AdamWillden commented on July 17, 2024

Thanks @crazycodr I do realise the issue and as soon as I hit the error I read up on the 30 char limit before posting as I haven't much dealt with oracle before. I do realise what you're saying about the naming conventions oracle users use and will converse with my knowledgeable colleague on this front to see if we can follow convention here.

We are moving an existing system to laravel and so we're adopting the migrations and want to use this extension for as much of the migrations as possible. I did not design the database and am not yet familiar enough with the existing system to know what can and can't be changed.

However, the fact remains that my primary column is not an autoIncrementing one, the trigger and sequence it is attempting to remove was never even created, so there's no need for the dropTrigger and dropSequence methods to be run and therefore no reason for the error to be produced.

Is it not possible to get the column definition of name $col from $blueprint and check $columnDefinition->autoIncrement? If what I'm asking is simply not possible then please by all means close the issue :-) I thought it'd be a small change to the code.

from laravel-oci8.

AdamWillden avatar AdamWillden commented on July 17, 2024

I thought this would be possible (I've just fudged it together in this window)

if (isset($col) and !empty($col)) {
    $columns = $blueprint->getColumns();

    // search for column definition
    foreach ($columns as $column) {
        // if column is found and is autoIncrement
        if (strcasecmp‎($col, $column->name) == 0 and $column->autoIncrement) {
            // drop sequence for auto increment
            $this->connection->dropSequence("{$prefix}{$table}_{$col}_seq");
            // drop trigger for auto increment work around
            $this->connection->dropTrigger("{$prefix}{$table}_{$col}_trg");
            break;
        }
    }
}

from laravel-oci8.

crazycodr avatar crazycodr commented on July 17, 2024

What i would recommend, if you can actually fix it:

  1. Fork the project
  2. Make the modification
  3. Submit a Pull Request

Then, @yajra will only have to accept it and merge it. I've also heard that it's possible in the composer.json to submit your own repositories and composer will look at your repository first and download your version. But i don't know how to do it. If you do manage it, you can fix the issue, push a PR for a future fix and in the meantime use your version without actually creating a packagist release (not recommended anyway)...

from laravel-oci8.

yajra avatar yajra commented on July 17, 2024

can you paste the migration script so I can play with it? Oracle does not have autoincrement, I just assume that a primary key indicated in the table is functioning most of the time like that. I do have a fallback to skip the drop trigger function but it seems to fail. I guess it needs some more tweaking
--EDITED: have not refresh the page and didn't see the long response.. ---

from laravel-oci8.

yajra avatar yajra commented on July 17, 2024

committed a fix that limits seq/trg name to 30chars. 44d9a0e

let me know if it works. thanks!

from laravel-oci8.

AdamWillden avatar AdamWillden commented on July 17, 2024

What I didn't realise/appreciate is that the drop blueprint has no information about the existing table columns. Therefore there's no way of knowing if there's an autoIncrement column.

I'm sure the 30char limit will do fine in this case. I will test it and report back.

With regards to the underlying issue that the table name is simply too long by convention I will converse with my colleague.

Many thanks for your help and patience.

from laravel-oci8.

AdamWillden avatar AdamWillden commented on July 17, 2024

Works great 👍

One thought on your implementation is that you don't HAVE to limit to 30 chars in the create/up migration and to just limit it in the drop/down migration to avoid my specific scenario. The reason I say this is that any user that wants to use auto increment could be forced by convention and the error would be thrown during the create/up migration so it would be obvious what the issue is rather than truncating a good conventional name.

Completely up to you of course 🎱 I do see the benefits to having it in both! I sincerely appreciate your efforts in helping me.

If you're happy with the solution please feel free to close this issue.

from laravel-oci8.

yajra avatar yajra commented on July 17, 2024

@AdamWillden, blueprint on drop indeed does not have information about the primary/autoincrement column. That is why I added a function to get the primary key using oracle specific query with assumptions that the primary key is autoincrement.

With regard to 30 chars limit, I guess I will stick to it since the autoincrement is just a hack I created so that some packages like Sentry, etc.. would work with this package. I have already implemented this 30 chars limit when creating the index name (fyi).

Lastly, I think I need to note this on read me that autoincrement support (sequence and trigger generated) and index name will be truncated to 30 chars.

from laravel-oci8.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.