1
\$\begingroup\$

I have developed this script to read employee_language table records of a PostgreSQL database using Python 2 (due to some OS limitation). I want to find duplicate language records for each employee, check if there are any duplicate language records for each employee, get the most completed language records among duplicated language records and finally return the ids of language records which are not duplicated with ids of most completed language records of duplicated ones (one records among many duplicates for every language record). This method works well for me but when I run this script for a great number of employee records (500K to 1000K) and it take a long time. Therefore, I'm wondering how I can make this method faster and restructure it better.

def read_employees_with_language_record(self):
    self.apps_cursor.execute("""
                                SELECT id FROM hr_employee limit 1000
                            """)
    employee_ids = self.apps_cursor.fetchall()
    counter = 0
    for employee_id in employee_ids:
        # Call method to get duplicate language records for employee
        language_records_not_be_deleted = self.get_employee_language_data(
            employee_id['id'])
        counter += 1
        # Call method to delete duplicate language records for employee
        self.delete_employee_duplicate_language_record(
            employee_id['id'], language_records_not_be_deleted)
        print 'Employee count ##############', counter

def get_employee_language_data(self, employee_id):
    self.apps_cursor.execute("SELECT id, language_id, hr_employee_id, lang_write, lang_speak, lang_read FROM employee_language WHERE hr_employee_id=%s" % employee_id)
    employee_languages = self.apps_cursor.fetchall()

    language_id_list = Counter(language.get('language_id')
                               for language in employee_languages)

    repeated_language_records = []
    non_repeated_language_records = []
    duplicate_language_ids_list = set()
    repeated_language_records_not_to_be_deleted = []

    for language in employee_languages:
        if language_id_list[language['language_id']] > 1:
            duplicate_language_ids_list.add(language['language_id'])
            repeated_language_records.append(language)
        else:
            non_repeated_language_records.append(language)

    for unique_record in duplicate_language_ids_list:
        temp_list = []
        flag_3 = False
        flag_2 = False
        flag_1 = False
        for repeated in repeated_language_records:

            if repeated['language_id'] == unique_record:

                if None not in (repeated['lang_speak'], repeated['lang_read'], repeated['lang_write']):
                    temp_list = repeated
                    flag_3 = True

                elif (repeated['lang_speak'], repeated['lang_read'], repeated['lang_write']).count(None) == 1 and not flag_3:
                    temp_list = repeated
                    flag_2 = True

                elif (repeated['lang_speak'], repeated['lang_read'], repeated['lang_write']).count(None) == 2 and not flag_3 and not flag_2:
                    temp_list = repeated
                    flag_1 = True

                elif not flag_3 and not flag_2 and not flag_1:
                    temp_list = repeated

        repeated_language_records_not_to_be_deleted.append(temp_list)

    language_record_ids_not_to_be_deleted = []

    for record in non_repeated_language_records + repeated_language_records_not_to_be_deleted:
        language_record_ids_not_to_be_deleted.append(int(record['id']))

    return language_record_ids_not_to_be_deleted

deleted_language_record_ids_list = []

def delete_employee_duplicate_language_record(self, hr_employee_id, record_ids):
    if record_ids:
        self.apps_cursor.execute("""
                                    SELECT id FROM employee_language WHERE hr_employee_id=%s AND id NOT IN %s
                                """, (hr_employee_id, tuple(record_ids),))
        deleted_language_records = self.apps_cursor.fetchall()
        for language_record in deleted_language_records:
            self.deleted_language_record_ids_list.append(
                language_record['id'])

        self.apps_cursor.execute("""
                                    DELETE FROM employee_language WHERE hr_employee_id=%s AND id NOT IN %s
                                """, (hr_employee_id, tuple(record_ids),))

def print_total_deleted_language_record(self):
    print 'Total deleted language records are: ', len(
        self.deleted_language_record_ids_list)
\$\endgroup\$
  • \$\begingroup\$ You could change your SQL query to already give you the count of each language \$\endgroup\$ – Tweakimp Dec 24 '19 at 14:43
  • \$\begingroup\$ You're right with count, bu I don't just need the count. I need to get the duplicate language records then among them, find and get the most completed one and delete others. So, I don't think count can help me in this case. \$\endgroup\$ – Ibrahim Rahimi Dec 25 '19 at 4:44
2
\$\begingroup\$

Looking at your code, I can see two potential slowdowns:

  1. When you do a .count, Python is implicitly performing a for-loop over each element in the array. This scales horribly with bigger tables and is definitely causing a slowdown. The comment about returning a count would actually help a lot. Below, I've implemented a similar solution.
  2. Converting a list to a set is usually fine, but in this case, when you have almost a million elements, it is not efficient. It is especially inefficient when there was no use for the list in the first place. From the code you gave, it seems like the best solution is to directly instantiate a set and add to that incrementally.

Implementing those points and then cleaning up the logic should result in something like this:

from collections import Counter


def get_employee_language_data(self, employee_id):
    self.apps_cursor.execute("SELECT id, language_id, hr_employee_id, lang_write, lang_speak, lang_read FROM employee_language WHERE hr_employee_id=%s" % employee_id)
    employee_languages = self.apps_cursor.fetchall()

    language_id_list = Counter(language.get('language_id') for language in employee_languages)

    repeated_language_records = []
    non_repeated_language_records = []
    duplicate_language_ids_list = set()
    repeated_language_records_not_to_be_deleted = []

    for language in employee_languages:
        if language_id_list[language['language_id']] > 1:
            duplicate_language_ids_list.add(language['language_id'])
            repeated_language_records.append(language)
        else:
            non_repeated_language_records.append(language)


    for unique_record in duplicate_language_ids_list:
        temp_list = []
        flag_3 = False
        flag_2 = False
        flag_1 = False
        for repeated in repeated_language_records:

            if repeated['language_id'] == unique_record:

                if None not in (repeated['lang_speak'], repeated['lang_read'], repeated['lang_write']):
                    temp_list = repeated
                    flag_3 = True

                elif (repeated['lang_speak'], repeated['lang_read'], repeated['lang_write']).count(None) == 1 and not flag_3:
                    temp_list = repeated
                    flag_2 = True

                elif (repeated['lang_speak'], repeated['lang_read'], repeated['lang_write']).count(None) == 1 and not flag_3 and not flag_2:
                    temp_list = repeated
                    flag_1 = True

                elif not flag_3 and not flag_2 and not flag_1:
                    temp_list = repeated

        repeated_language_records_not_to_be_deleted.append(temp_list)

    language_record_ids_not_to_be_deleted = []

    for record in non_repeated_language_records + repeated_language_records_not_to_be_deleted:
        language_record_ids_not_to_be_deleted.append(int(record['id']))

    return language_record_ids_not_to_be_deleted

I'm sure there are further improvements that could be made, but I cannot follow the specific logic of the code you gave. More context and examples would be necessary to simplify it further. I am certain this is faster, but as fast as it could be.

|improve this answer|||||
\$\endgroup\$
  • \$\begingroup\$ I applied above changes in my code and test it with measuring the script run time with original script. With testing for several scenarios, these changes made the script even slower then before. I'm still wonder how can make this script faster and I tired some methods but nothing changed a lot. \$\endgroup\$ – Ibrahim Rahimi Jan 5 at 4:29
  • \$\begingroup\$ @IbrahimRahimi Can you post your updated code? \$\endgroup\$ – Zchpyvr Jan 5 at 16:13
  • \$\begingroup\$ It's too long and can't be fit in a comment. Should I post it as an answer? \$\endgroup\$ – Ibrahim Rahimi Jan 6 at 4:38
  • \$\begingroup\$ @IbrahimRahimi, no please post it as an edit to your answer. \$\endgroup\$ – Zchpyvr Jan 6 at 15:49
  • \$\begingroup\$ I have posted it as an edit for the answer. Please review it and let me know I can improve it any way. \$\endgroup\$ – Ibrahim Rahimi Jan 11 at 7:54

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.