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

Crash while using Oracle views with primary key defined as number without precision #31626

Closed
juanmpd opened this issue Sep 9, 2019 · 2 comments
Closed

Comments

@juanmpd
Copy link
Contributor

@juanmpd juanmpd commented Sep 9, 2019

We have found a situation that makes QGis Crash or have unpredictable behavior. We have tested it on 3.4 nightly, and on 3.9 Master. The problem shows in both of them. Its source is related to allowing creation of layers mapping Oracle table/views with primary keys of datatype number, without precision, which are handled as float. We wonder if they should not be allowed at all.
As it has been hard to find what was behind the crash (some man days), we think it would be safer if QGis disallowed that situation, giving a controlled error message, instead of later crashing or behave unpredictably. Curiously, under QGIS 2.18 it did not crash.
In our tests to reproduce the issue, QGis behavior is not fully predictable. It may crash, or it may not. In some of the scenarios we have faced, it seems to behave alternating crash and not crash. Although when it does not crash, the results are also unpredictable.
The problem seems related to having a layer mapped against an Oracle view whose primary key is an Oracle column of number datatype, where it has been declared without precision (just as plain ‘number’). QGis disallows interactively creating layers against Oracle tables with such primary keys. However, it does allow interactively or programmatically creating layers against views with such primary keys. We have not checked if it allows programmatically creating layers against tables with such primary keys.

How to reproduce - Step 1: prepare the Oracle view
You need access to an Oracle database where you are able to create a table and a view. Then you can use the attached ‘oracle_setup.sql’ to create a table with ‘number’ primary key, and a view over this table:

create table TABLE_QGIS_ISSUE_FLOAT_KEY(
    CODE    NUMBER PRIMARY KEY,
    DESCRIPTION VARCHAR2(25)
);

insert into TABLE_QGIS_ISSUE_FLOAT_KEY values(1000,'Desc for 1st record');
insert into TABLE_QGIS_ISSUE_FLOAT_KEY values(2000,'Desc for 2nd record');
commit;

create view VIEW_QGIS_ISSUE_FLOAT_KEY as select * from TABLE_QGIS_ISSUE_FLOAT_KEY;

How to reproduce - Step 2: have a QGis project with a layer pointing to this Oracle view
Steps:

  • Start QGis

  • Follow the steps to add an Oracle layer (main menu > layer > add layer > add Oracle spatial layer)

    0 1 add oracle layer step 1

  • The dialog for managing Oracle data sources will show up.

  • Press ‘New’ to create a new Oracle connection. Fill in the input fields as needed. Be sure to check the option to also list tables without geometry (as the sql we have provided to demonstrate the problem creates a table without geometry, just for simplicity).

    1-create_oracle_connection

  • Once the connection is defined, use it to add a new layer to VIEW_QGIS_ISSUE_FLOAT_KEY, specifying to use ‘CODE’ column as primary key.

    2-adding_oracle_layer

How to reproduce - Step 3: just try to access the features using the provided python script
We have prepared a simple python script that simply accesses the features in the just created layer.

LAYER_NAME='VIEW_QGIS_ISSUE_FLOAT_KEY'
KEY_COLUMN_NAME='CODE'


print("THIS TEST IS INTENDED TO BE RUN JUST AFTER RESTARTING QGIS AND THEN LOADING THE QGIS PROJECT / ADDING THE LAYER")
l=QgsProject.instance().mapLayersByName(LAYER_NAME)[0]

def k0():
    id = 2

    print("getFeature({})".format(id))
    f = l.getFeature(id)
    printInfo(f)
    
    cnt = 2
    fs = l.getFeatures()
    while True:
        print("next(getFeatures())")
        f = next(fs)
        printInfo(f)
        cnt -= 1
        if cnt<= 0:
            break
    
    print("getFeature({})".format(id))
    f = l.getFeature(id)
    printInfo(f)

def printInfo(f):
    if f.isValid():
        oracleKey = f[KEY_COLUMN_NAME]
    else:
        oracleKey = "NOT VALID"
    print(" {}".format(f))
    print(" QGis Id: {} --> Oracle Key: {}".format(f.id(), oracleKey))

k0()

This scripts shows the unpredictable behavior:

  • Either it will make crash QGis when invoking getFeature() for a not existing feature for the first time.
    3-crash

  • Either it will show getFeature() returning different values when invoked for a not existing feature for the first time and then again after iterating over the actual features.
    3 - Incongruent

We suppose the problems come from the fact that QGis refuses using the given primary key column as identifier, because it considers it as float, because it was declared without precision. If the column was declared as number(38,0), for example, then no problems arises. In fact, the solution in our case has been changing the oracle view definition to include a “cast as number(38,0)”. Nonetheless, though we agree that QGis should not allow for ‘float’ primary keys, we think QGis should not allow the layer to be created at all, to avoid crashes or inconsistent results.

@troopa81

This comment has been minimized.

Copy link
Contributor

@troopa81 troopa81 commented Feb 3, 2020

@juanmpd I'm not 100% sure this is an issue.

I manage to reproduce the issue and QGIS crash because you try to get the feature id 2 when it doesn't actually exist.

If you remove the first getFeature(id), it works all the time.

The explanation is that when you use float column (or string) as a primary key column, QGIS cannot use this as id. Id has to be an integer (QgsFeatureId is basically an qint64). So QGIS builds a mapping between the column value (float or string) and the id (starting from 1 and incrementing for each feature it reads).

the mapping is lazily computed while you read the feature, but in your scenario, you never open the attribute table or render the feature, so you never read the feature. And so feature id 2 doesn't exist when you write getFeature(2). QGIS cannot build an oracle filter and crash.

IMHO, in your scenario, you should use getFeature with a request based on an expression on your column CODE

I still can propose a fix to avoid crash and just return an INVALID feature on the first getFeature.

@troopa81 troopa81 added the Feedback label Feb 3, 2020
@juanmpd

This comment has been minimized.

Copy link
Contributor Author

@juanmpd juanmpd commented Feb 4, 2020

I think any fix that could prevent the crash would be fine...
Thank you very much!

@troopa81 troopa81 removed the Feedback label Feb 5, 2020
troopa81 added a commit to troopa81/QGIS that referenced this issue Feb 5, 2020
troopa81 added a commit to troopa81/QGIS that referenced this issue Feb 5, 2020
nyalldawson added a commit that referenced this issue Feb 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.